Deploy and proposal for USDT market on Goerli

.github/workflows/run-scenarios.yaml

@@ -7,7 +7,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        bases: [ development, mainnet, mainnet-weth, goerli, goerli-weth, fuji, mumbai, polygon, arbitrum-usdc.e, arbitrum-usdc, arbitrum-goerli-usdc, arbitrum-goerli-usdc.e, base-usdbc, base-weth, base-goerli, base-goerli-weth, linea-goerli]
+        bases: [ development, mainnet, mainnet-weth, goerli, goerli-weth, goerli-usdt, fuji, mumbai, polygon, arbitrum-usdc.e, arbitrum-usdc, arbitrum-goerli-usdc, arbitrum-goerli-usdc.e, base-usdbc, base-weth, base-goerli, base-goerli-weth, linea-goerli]
     name: Run scenarios
     env:
       ETHERSCAN_KEY: ${{ secrets.ETHERSCAN_KEY }}

contracts/Comet.sol

@@ -2,15 +2,16 @@
 pragma solidity 0.8.15;
 
 import "./CometMainInterface.sol";
-import "./ERC20.sol";
+import "./IERC20NonStandard.sol";
 import "./IPriceFeed.sol";
+import "./ReentrancyGuard.sol";
 
 /**
  * @title Compound's Comet Contract
  * @notice An efficient monolithic money market protocol
  * @author Compound
  */
-contract Comet is CometMainInterface {
+contract Comet is CometMainInterface, ReentrancyGuard {
     /** General configuration constants **/
 
     /// @notice The admin of the protocol
@@ -139,7 +140,7 @@
      **/
     constructor(Configuration memory config) {
         // Sanity checks
-        uint8 decimals_ = ERC20(config.baseToken).decimals();
+        uint8 decimals_ = IERC20NonStandard(config.baseToken).decimals();
         if (decimals_ > MAX_BASE_DECIMALS) revert BadDecimals();
         if (config.storeFrontPriceFactor > FACTOR_SCALE) revert BadDiscount();
         if (config.assetConfigs.length > MAX_ASSETS) revert TooManyAssets();
@@ -224,7 +225,7 @@
     function getPackedAssetInternal(AssetConfig[] memory assetConfigs, uint i) internal view returns (uint256, uint256) {
         AssetConfig memory assetConfig;
         if (i < assetConfigs.length) {
             assembly {
                 assetConfig := mload(add(add(assetConfigs, 0x20), mul(i, 0x20)))
             }
         } else {
@@ -241,7 +242,7 @@
 
         // Sanity check price feed and asset decimals
         if (IPriceFeed(priceFeed).decimals() != PRICE_FEED_DECIMALS) revert BadDecimals();
-        if (ERC20(asset).decimals() != decimals_) revert BadDecimals();
+        if (IERC20NonStandard(asset).decimals() != decimals_) revert BadDecimals();
 
         // Ensure collateral factors are within range
         if (assetConfig.borrowCollateralFactor >= assetConfig.liquidateCollateralFactor) revert BorrowCFTooLarge();
@@ -482,15 +483,15 @@
      * @param asset The collateral asset
      */
     function getCollateralReserves(address asset) override public view returns (uint) {
-        return ERC20(asset).balanceOf(address(this)) - totalsCollateral[asset].totalSupplyAsset;
+        return IERC20NonStandard(asset).balanceOf(address(this)) - totalsCollateral[asset].totalSupplyAsset;
     }
 
     /**
      * @notice Gets the total amount of protocol reserves of the base asset
      */
     function getReserves() override public view returns (int) {
         (uint64 baseSupplyIndex_, uint64 baseBorrowIndex_) = accruedInterestIndices(getNowInternal() - lastAccrualTime);
-        uint balance = ERC20(baseToken).balanceOf(address(this));
+        uint balance = IERC20NonStandard(baseToken).balanceOf(address(this));
         uint totalSupply_ = presentValueSupply(baseSupplyIndex_, totalSupplyBase);
         uint totalBorrow_ = presentValueBorrow(baseBorrowIndex_, totalBorrowBase);
         return signed256(balance) - signed256(totalSupply_) + signed256(totalBorrow_);
@@ -760,18 +761,50 @@
     }
 
     /**
-     * @dev Safe ERC20 transfer in, assumes no fee is charged and amount is transferred
+     * @dev Safe ERC20 transfer in and returns the final amount transferred (taking into account any fees)
+     * @dev Note: Safely handles non-standard ERC-20 tokens that do not return a value. See here: https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca
      */
-    function doTransferIn(address asset, address from, uint amount) internal {
-        bool success = ERC20(asset).transferFrom(from, address(this), amount);
+    function doTransferIn(address asset, address from, uint amount) internal returns (uint) {
+        uint256 preTransferBalance = IERC20NonStandard(asset).balanceOf(address(this));
+        IERC20NonStandard(asset).transferFrom(from, address(this), amount);
+        bool success;
+        assembly {
+            switch returndatasize()
+                case 0 {                       // This is a non-standard ERC-20
+                    success := not(0)          // set success to true
+                }
+                case 32 {                      // This is a compliant ERC-20
+                    returndatacopy(0, 0, 32)
+                    success := mload(0)        // Set `success = returndata` of override external call
+                }
+                default {                      // This is an excessively non-compliant ERC-20, revert.
+                    revert(0, 0)
+                }
+        }
         if (!success) revert TransferInFailed();
+        return IERC20NonStandard(asset).balanceOf(address(this)) - preTransferBalance;
     }
 
     /**
      * @dev Safe ERC20 transfer out
+     * @dev Note: Safely handles non-standard ERC-20 tokens that do not return a value. See here: https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca
      */
     function doTransferOut(address asset, address to, uint amount) internal {
-        bool success = ERC20(asset).transfer(to, amount);
+        IERC20NonStandard(asset).transfer(to, amount);
+        bool success;
+        assembly {
+            switch returndatasize()
+                case 0 {                       // This is a non-standard ERC-20
+                    success := not(0)          // set success to true
+                }
+                case 32 {                      // This is a compliant ERC-20
+                    returndatacopy(0, 0, 32)
+                    success := mload(0)        // Set `success = returndata` of override external call
+                }
+                default {                      // This is an excessively non-compliant ERC-20, revert.
+                    revert(0, 0)
+                }
+        }
         if (!success) revert TransferOutFailed();
     }
 
@@ -809,7 +842,7 @@
      * @dev Supply either collateral or base asset, depending on the asset, if operator is allowed
      * @dev Note: Specifying an `amount` of uint256.max will repay all of `dst`'s accrued base borrow balance
      */
-    function supplyInternal(address operator, address from, address dst, address asset, uint amount) internal {
+    function supplyInternal(address operator, address from, address dst, address asset, uint amount) internal nonReentrant {
         if (isSupplyPaused()) revert Paused();
         if (!hasPermission(from, operator)) revert Unauthorized();
 
@@ -827,7 +860,7 @@
      * @dev Supply an amount of base asset from `from` to dst
      */
     function supplyBase(address from, address dst, uint256 amount) internal {
-        doTransferIn(baseToken, from, amount);
+        amount = doTransferIn(baseToken, from, amount);
 
         accrueInternal();
 
@@ -854,7 +887,7 @@
      * @dev Supply an amount of collateral asset from `from` to dst
      */
     function supplyCollateral(address from, address dst, address asset, uint128 amount) internal {
-        doTransferIn(asset, from, amount);
+        amount = safe128(doTransferIn(asset, from, amount));
 
         AssetInfo memory assetInfo = getAssetInfoByAddress(asset);
         TotalsCollateral memory totals = totalsCollateral[asset];
@@ -920,7 +953,7 @@
      * @dev Transfer either collateral or base asset, depending on the asset, if operator is allowed
      * @dev Note: Specifying an `amount` of uint256.max will transfer all of `src`'s accrued base balance
      */
-    function transferInternal(address operator, address src, address dst, address asset, uint amount) internal {
+    function transferInternal(address operator, address src, address dst, address asset, uint amount) internal nonReentrant {
         if (isTransferPaused()) revert Paused();
         if (!hasPermission(src, operator)) revert Unauthorized();
         if (src == dst) revert NoSelfTransfer();
@@ -1031,7 +1064,7 @@
      * @dev Withdraw either collateral or base asset, depending on the asset, if operator is allowed
      * @dev Note: Specifying an `amount` of uint256.max will withdraw all of `src`'s accrued base balance
      */
-    function withdrawInternal(address operator, address src, address to, address asset, uint amount) internal {
+    function withdrawInternal(address operator, address src, address to, address asset, uint amount) internal nonReentrant {
         if (isWithdrawPaused()) revert Paused();
         if (!hasPermission(src, operator)) revert Unauthorized();
 
@@ -1192,14 +1225,14 @@
      * @param baseAmount The amount of base tokens used to buy the collateral
      * @param recipient The recipient address
      */
-    function buyCollateral(address asset, uint minAmount, uint baseAmount, address recipient) override external {
+    function buyCollateral(address asset, uint minAmount, uint baseAmount, address recipient) override external nonReentrant {
         if (isBuyPaused()) revert Paused();
 
         int reserves = getReserves();
         if (reserves >= 0 && uint(reserves) >= targetReserves) revert NotForSale();
 
         // Note: Re-entrancy can skip the reserves check above on a second buyCollateral call.
-        doTransferIn(baseToken, msg.sender, baseAmount);
+        baseAmount = doTransferIn(baseToken, msg.sender, baseAmount);
 
         uint collateralAmount = quoteCollateral(asset, baseAmount);
         if (collateralAmount < minAmount) revert TooMuchSlippage();
@@ -1238,7 +1271,7 @@
      * @param to An address of the receiver of withdrawn reserves
      * @param amount The amount of reserves to be withdrawn from the protocol
      */
-    function withdrawReserves(address to, uint amount) override external {
+    function withdrawReserves(address to, uint amount) override external nonReentrant {
         if (msg.sender != governor) revert Unauthorized();
 
         int reserves = getReserves();
@@ -1261,7 +1294,7 @@
     function approveThis(address manager, address asset, uint amount) override external {
         if (msg.sender != governor) revert Unauthorized();
 
-        ERC20(asset).approve(manager, amount);
+        IERC20NonStandard(asset).approve(manager, amount);
     }
 
     /**
@@ -1311,9 +1344,9 @@
     /**
      * @notice Fallback to calling the extension delegate for everything else
      */
     fallback() external payable {
         address delegate = extensionDelegate;
         assembly {
             calldatacopy(0, 0, calldatasize())
             let result := delegatecall(gas(), delegate, 0, calldatasize(), 0, 0)
             returndatacopy(0, 0, returndatasize())
@@ -1322,4 +1355,4 @@
             default { return(0, returndatasize()) }
         }
     }
-}
+}

contracts/IERC20NonStandard.sol

@@ -7,6 +7,7 @@ pragma solidity 0.8.15;
  *  See https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca
  */
 interface IERC20NonStandard {
+    function decimals() external view returns (uint8);
     function approve(address spender, uint256 amount) external;
     function transfer(address to, uint256 value) external;
     function transferFrom(address from, address to, uint256 value) external;

contracts/ReentrancyGuard.sol

@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.9.0) (security/ReentrancyGuard.sol)
+
+pragma solidity ^0.8.0;
+
+/**
+ * @dev Contract module that helps prevent reentrant calls to a function.
+ *
+ * Inheriting from `ReentrancyGuard` will make the {nonReentrant} modifier
+ * available, which can be applied to functions to make sure there are no nested
+ * (reentrant) calls to them.
+ *
+ * Note that because there is a single `nonReentrant` guard, functions marked as
+ * `nonReentrant` may not call one another. This can be worked around by making
+ * those functions `private`, and then adding `external` `nonReentrant` entry
+ * points to them.
+ *
+ * TIP: If you would like to learn more about reentrancy and alternative ways
+ * to protect against it, check out our blog post
+ * https://blog.openzeppelin.com/reentrancy-after-istanbul/[Reentrancy After Istanbul].
+ */
+abstract contract ReentrancyGuard {
+    // Booleans are more expensive than uint256 or any type that takes up a full
+    // word because each write operation emits an extra SLOAD to first read the
+    // slot's contents, replace the bits taken up by the boolean, and then write
+    // back. This is the compiler's defense against contract upgrades and
+    // pointer aliasing, and it cannot be disabled.
+
+    // The values being non-zero value makes deployment a bit more expensive,
+    // but in exchange the refund on every call to nonReentrant will be lower in
+    // amount. Since refunds are capped to a percentage of the total
+    // transaction's gas, it is best to keep them low in cases like this one, to
+    // increase the likelihood of the full refund coming into effect.
+    uint256 private constant _NOT_ENTERED = 1;
+    uint256 private constant _ENTERED = 2;
+
+    uint256 private _status;
+
+    constructor() {
+        _status = _NOT_ENTERED;
+    }
+
+    /**
+     * @dev Prevents a contract from calling itself, directly or indirectly.
+     * Calling a `nonReentrant` function from another `nonReentrant`
+     * function is not supported. It is possible to prevent this from happening
+     * by making the `nonReentrant` function external, and making it call a
+     * `private` function that does the actual work.
+     */
+    modifier nonReentrant() {
+        _nonReentrantBefore();
+        _;
+        _nonReentrantAfter();
+    }
+
+    function _nonReentrantBefore() private {
+        // On the first call to nonReentrant, _status will be _NOT_ENTERED
+        require(_status != _ENTERED, "ReentrancyGuard: reentrant call");
+
+        // Any calls to nonReentrant after this point will fail
+        _status = _ENTERED;
+    }
+
+    function _nonReentrantAfter() private {
+        // By storing the original value once again, a refund is triggered (see
+        // https://eips.ethereum.org/EIPS/eip-2200)
+        _status = _NOT_ENTERED;
+    }
+
+    /**
+     * @dev Returns true if the reentrancy guard is currently set to "entered", which indicates there is a
+     * `nonReentrant` function in the call stack.
+     */
+    function _reentrancyGuardEntered() internal view returns (bool) {
+        return _status == _ENTERED;
+    }
+}

contracts/test/EvilToken.sol

@@ -52,20 +52,27 @@ contract EvilToken is FaucetToken {
         attack = attack_;
     }
 
-    function transfer(address, uint256) external override returns (bool) {
-        return performAttack();
+    function transfer(address dst, uint256 amount) public override returns (bool) {
+        numberOfCalls++;
+        if (numberOfCalls > attack.maxCalls){
+            return super.transfer(dst, amount);
+        } else {
+            return performAttack(address(this), dst, amount);
+        }
     }
 
-    function transferFrom(address, address, uint256) external override returns (bool) {
-        return performAttack();
+    function transferFrom(address src, address dst, uint256 amount) public override returns (bool) {
+        numberOfCalls++;
+        if (numberOfCalls > attack.maxCalls) {
+            return super.transferFrom(src, dst, amount);
+        } else {
+            return performAttack(src, dst, amount);
+        }
     }
 
-    function performAttack() internal returns (bool) {
+    function performAttack(address src, address dst, uint256 amount) internal returns (bool) {
         ReentryAttack memory reentryAttack = attack;
-        numberOfCalls++;
-        if (numberOfCalls > reentryAttack.maxCalls) {
-            // do nothing
-        } else if (reentryAttack.attackType == AttackType.TRANSFER_FROM) {
+       if (reentryAttack.attackType == AttackType.TRANSFER_FROM) {
             Comet(payable(msg.sender)).transferFrom(
                 reentryAttack.source,
                 reentryAttack.destination,

contracts/test/FaucetToken.sol

@@ -24,15 +24,15 @@ contract StandardToken {
         decimals = _decimalUnits;
     }
 
-    function transfer(address dst, uint256 amount) external virtual returns (bool) {
+    function transfer(address dst, uint256 amount) public virtual returns (bool) {
         require(amount <= balanceOf[msg.sender], "ERC20: transfer amount exceeds balance");
         balanceOf[msg.sender] = balanceOf[msg.sender] - amount;
         balanceOf[dst] = balanceOf[dst] + amount;
         emit Transfer(msg.sender, dst, amount);
         return true;
     }
 
-    function transferFrom(address src, address dst, uint256 amount) external virtual returns (bool) {
+    function transferFrom(address src, address dst, uint256 amount) public virtual returns (bool) {
         require(amount <= allowance[src][msg.sender], "ERC20: transfer amount exceeds allowance");
         require(amount <= balanceOf[src], "ERC20: transfer amount exceeds balance");
         allowance[src][msg.sender] = allowance[src][msg.sender] - amount;