OSC Minor Fixes Combined

packages/contracts/contracts/CollateralManager.sol

@@ -18,7 +18,16 @@ import { Collateral, CollateralType, ICollateralEscrowV1 } from "./interfaces/es
 import "./interfaces/ITellerV2.sol";
 import "./interfaces/IProtocolPausingManager.sol";
 import "./interfaces/IHasProtocolPausingManager.sol";
+
+import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
+import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+
+
+
 contract CollateralManager is OwnableUpgradeable, ICollateralManager {
+
+    using SafeERC20 for ERC20;
+
     /* Storage */
     using EnumerableSetUpgradeable for EnumerableSetUpgradeable.AddressSet;
     ITellerV2 public tellerV2;
@@ -138,7 +147,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
     ) public onlyTellerV2 returns (bool validation_) {
         address borrower = tellerV2.getLoanBorrower(_bidId);
         require(borrower != address(0), "Loan has no borrower");
-        (validation_, ) = checkBalances(borrower, _collateralInfo);
+        (validation_, ) = _checkBalances(borrower, _collateralInfo, false);
 
         //if the collateral info is valid, call commitCollateral for each one
         if (validation_) {
@@ -149,24 +158,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
         }
     }
 
-    /**
-     * @notice Checks the validity of a borrower's collateral balance and commits it to a bid.
-     * @param _bidId The id of the associated bid.
-     * @param _collateralInfo Additional information about the collateral asset.
-     * @return validation_ Boolean indicating if the collateral balance was validated.
-     */
-    function commitCollateral(
-        uint256 _bidId,
-        Collateral calldata _collateralInfo
-    ) public onlyTellerV2 returns (bool validation_) {
-        address borrower = tellerV2.getLoanBorrower(_bidId);
-        require(borrower != address(0), "Loan has no borrower");
-        validation_ = _checkBalance(borrower, _collateralInfo);
-        if (validation_) {
-            _commitCollateral(_bidId, _collateralInfo);
-        }
-    }
-
+
     /**
      * @notice Re-checks the validity of a borrower's collateral balance committed to a bid.
      * @param _bidId The id of the associated bid.
@@ -199,8 +191,9 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
      */
     function deployAndDeposit(uint256 _bidId) external onlyTellerV2 {
         if (isBidCollateralBacked(_bidId)) {
-            //attempt deploy a new collateral escrow contract if there is not already one. Otherwise fetch it.
-            (address proxyAddress, ) = _deployEscrow(_bidId);
+            //attempt deploy a new collateral escrow contract for this bid if there is not already one. Otherwise fetch it.
+            (address proxyAddress, address borrower) = _deployEscrow(_bidId);
+
             _escrows[_bidId] = proxyAddress;
 
             //for each bid collateral associated with this loan, deposit the collateral into escrow
@@ -213,7 +206,9 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
                     _bidId,
                     _bidCollaterals[_bidId].collateralInfo[
                         _bidCollaterals[_bidId].collateralAddresses.at(i)
-                    ]
+                    ],
+                    proxyAddress,
+                    borrower 
                 );
             }
 
@@ -294,23 +289,7 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
                 );
         }
 
-    /**
-     * @notice Withdraws deposited collateral from the created escrow of a bid that has been CLOSED after being defaulted.
-     * @param _bidId The id of the bid to withdraw collateral for.
-     */
-    function lenderClaimCollateral(uint256 _bidId) external onlyTellerV2 whenProtocolNotPaused {
-        if (isBidCollateralBacked(_bidId)) {
-            BidState bidState = tellerV2.getBidState(_bidId);
-
-            require(
-                bidState == BidState.CLOSED,
-                "Loan has not been liquidated"
-            );
-
-            _withdraw(_bidId, tellerV2.getLoanLender(_bidId));
-            emit CollateralClaimed(_bidId);
-        }
-    }
+
 
         /**
      * @notice Withdraws deposited collateral from the created escrow of a bid that has been CLOSED after being defaulted.
@@ -385,18 +364,18 @@ contract CollateralManager is OwnableUpgradeable, ICollateralManager {
         * @param collateralInfo The collateral info to deposit.
 
     */
-    function _deposit(uint256 _bidId, Collateral memory collateralInfo)
+    function _deposit(uint256 _bidId, Collateral memory collateralInfo, address escrowAddress, address borrower  )
         internal
         virtual
     {
         require(collateralInfo._amount > 0, "Collateral not validated");
-        (address escrowAddress, address borrower) = _deployEscrow(_bidId);
+
         ICollateralEscrowV1 collateralEscrow = ICollateralEscrowV1(
             escrowAddress
         );
         // Pull collateral from borrower & deposit into escrow
         if (collateralInfo._collateralType == CollateralType.ERC20) {
-            IERC20Upgradeable(collateralInfo._collateralAddress).transferFrom(
+            ERC20(collateralInfo._collateralAddress).safeTransferFrom(
                 borrower,
                 address(this),
                 collateralInfo._amount

packages/contracts/contracts/MarketRegistry.sol

@@ -294,28 +294,7 @@ contract MarketRegistry is
         _attestStakeholder(_marketId, _lenderAddress, _expirationTime, true);
     }
 
-    /**
-     * @notice Adds a lender to a market via delegated attestation.
-     * @dev See {_attestStakeholderViaDelegation}.
-     */
-    function attestLender(
-        uint256 _marketId,
-        address _lenderAddress,
-        uint256 _expirationTime,
-        uint8 _v,
-        bytes32 _r,
-        bytes32 _s
-    ) external {
-        _attestStakeholderViaDelegation(
-            _marketId,
-            _lenderAddress,
-            _expirationTime,
-            true,
-            _v,
-            _r,
-            _s
-        );
-    }
+
 
     /**
      * @notice Removes a lender from an market.
@@ -324,28 +303,7 @@ contract MarketRegistry is
     function revokeLender(uint256 _marketId, address _lenderAddress) external {
         _revokeStakeholder(_marketId, _lenderAddress, true);
     }
-
-    /**
-     * @notice Removes a borrower from a market via delegated revocation.
-     * @dev See {_revokeStakeholderViaDelegation}.
-     */
-   /* function revokeLender(
-        uint256 _marketId,
-        address _lenderAddress,
-        uint8 _v,
-        bytes32 _r,
-        bytes32 _s
-    ) external {
-        _revokeStakeholderViaDelegation(
-            _marketId,
-            _lenderAddress,
-            true,
-            _v,
-            _r,
-            _s
-        );
-    } */
-
+
     /**
      * @notice Allows a lender to voluntarily leave a market.
      * @param _marketId The market ID to leave.
@@ -372,28 +330,7 @@ contract MarketRegistry is
         _attestStakeholder(_marketId, _borrowerAddress, _expirationTime, false);
     }
 
-    /**
-     * @notice Adds a borrower to a market via delegated attestation.
-     * @dev See {_attestStakeholderViaDelegation}.
-     */
-    function attestBorrower(
-        uint256 _marketId,
-        address _borrowerAddress,
-        uint256 _expirationTime,
-        uint8 _v,
-        bytes32 _r,
-        bytes32 _s
-    ) external {
-        _attestStakeholderViaDelegation(
-            _marketId,
-            _borrowerAddress,
-            _expirationTime,
-            false,
-            _v,
-            _r,
-            _s
-        );
-    }
+
 
     /**
      * @notice Removes a borrower from an market.
@@ -405,28 +342,7 @@ contract MarketRegistry is
         _revokeStakeholder(_marketId, _borrowerAddress, false);
     }
 
-    /**
-     * @notice Removes a borrower from a market via delegated revocation.
-     * @dev See {_revokeStakeholderViaDelegation}.
-     */
-   /* function revokeBorrower(
-        uint256 _marketId,
-        address _borrowerAddress,
-        uint8 _v,
-        bytes32 _r,
-        bytes32 _s
-    ) external {
-        _revokeStakeholderViaDelegation(
-            _marketId,
-            _borrowerAddress,
-            false,
-            _v,
-            _r,
-            _s
-        );
-    }*/
-
-    /**
+    /*
      * @notice Allows a borrower to voluntarily leave a market.
      * @param _marketId The market ID to leave.
      */
@@ -1063,58 +979,7 @@ contract MarketRegistry is
             _isLender
         );
     }
-
-    /**
-     * @notice Adds a stakeholder (lender or borrower) to a market via delegated attestation.
-     * @dev The signature must match that of the market owner.
-     * @param _marketId The market ID to add a lender to.
-     * @param _stakeholderAddress The address of the lender to add to the market.
-     * @param _expirationTime The expiration time of the attestation.
-     * @param _isLender Boolean indicating if the stakeholder is a lender. Otherwise it is a borrower.
-     * @param _v Signature value
-     * @param _r Signature value
-     * @param _s Signature value
-     */
-    function _attestStakeholderViaDelegation(
-        uint256 _marketId,
-        address _stakeholderAddress,
-        uint256 _expirationTime,
-        bool _isLender,
-        uint8 _v,
-        bytes32 _r,
-        bytes32 _s
-    )
-        internal
-        virtual
-        withAttestingSchema(
-            _isLender ? lenderAttestationSchemaId : borrowerAttestationSchemaId
-        )
-    {
-        // NOTE: block scope to prevent stack too deep!
-        bytes32 uuid;
-        {
-            bytes memory data = abi.encode(_marketId, _stakeholderAddress);
-            address attestor = _getMarketOwner(_marketId);
-            // Submit attestation for stakeholder to join a market (attestation must be signed by market owner)
-            uuid = tellerAS.attestByDelegation(
-                _stakeholderAddress,
-                _attestingSchemaId, // set by the modifier
-                _expirationTime,
-                0,
-                data,
-                attestor,
-                _v,
-                _r,
-                _s
-            );
-        }
-        _attestStakeholderVerification(
-            _marketId,
-            _stakeholderAddress,
-            uuid,
-            _isLender
-        );
-    }
+
 
     /**
      * @notice Adds a stakeholder (borrower/lender) to a market.
@@ -1180,25 +1045,7 @@ contract MarketRegistry is
         //        tellerAS.revoke(uuid);
     }
 
-
-   /* function _revokeStakeholderViaDelegation(
-        uint256 _marketId,
-        address _stakeholderAddress,
-        bool _isLender,
-        uint8 _v,
-        bytes32 _r,
-        bytes32 _s
-    ) internal {
-        bytes32 uuid = _revokeStakeholderVerification(
-            _marketId,
-            _stakeholderAddress,
-            _isLender
-        );
-        // NOTE: Disabling the call to revoke the attestation on EAS contracts
-        //        address attestor = markets[_marketId].owner;
-        //        tellerAS.revokeByDelegation(uuid, attestor, _v, _r, _s);
-    } */
-
+
     /**
      * @notice Removes a stakeholder (borrower/lender) from a market.
      * @param _marketId The market ID to remove the lender from.

packages/contracts/contracts/escrow/CollateralEscrowV1.sol

@@ -54,7 +54,7 @@ contract CollateralEscrowV1 is OwnableUpgradeable, ICollateralEscrowV1 {
         address _collateralAddress,
         uint256 _amount,
         uint256 _tokenId
-    ) external payable virtual onlyOwner {
+    ) external virtual onlyOwner {
         require(_amount > 0, "Deposit amount cannot be zero");
         _depositCollateral(
             _collateralType,
@@ -93,13 +93,17 @@ contract CollateralEscrowV1 is OwnableUpgradeable, ICollateralEscrowV1 {
             collateral._amount >= _amount,
             "No collateral balance for asset"
         );
+
+        //this comes first to mitigate re-entrancy via checks-effects-interact pattern
+        collateral._amount -= _amount;
+
         _withdrawCollateral(
             collateral,
             _collateralAddress,
             _amount,
             _recipient
         );
-        collateral._amount -= _amount;
+
         emit CollateralWithdrawn(_collateralAddress, _amount, _recipient);
     }
 

packages/contracts/contracts/interfaces/ICollateralManager.sol

@@ -15,17 +15,7 @@ interface ICollateralManager {
         Collateral[] calldata _collateralInfo
     ) external returns (bool validation_);
 
-    /**
-     * @notice Checks the validity of a borrower's collateral balance and commits it to a bid.
-     * @param _bidId The id of the associated bid.
-     * @param _collateralInfo Additional information about the collateral asset.
-     * @return validation_ Boolean indicating if the collateral balance was validated.
-     */
-    function commitCollateral(
-        uint256 _bidId,
-        Collateral calldata _collateralInfo
-    ) external returns (bool validation_);
-
+
     function checkBalances(
         address _borrowerAddress,
         Collateral[] calldata _collateralInfo
@@ -72,14 +62,7 @@ interface ICollateralManager {
      */
     function revalidateCollateral(uint256 _bidId) external returns (bool);
 
-    /**
-     * @notice Sends the deposited collateral to a lender of a bid.
-     * @notice Can only be called by the protocol.
-     * @param _bidId The id of the liquidated bid.
-     */
-    function lenderClaimCollateral(uint256 _bidId) external;
-
-
+
 
     /**
      * @notice Sends the deposited collateral to a lender of a bid.