PoolTogether micro contest #1 - hrkrshnn's results

Handle

hrkrshnn

Vulnerability details

Function that can be made external

MStableYieldSource.sol

1. function approveMax() public
2. function depositToken() public

The above two functions can be converted to external.

Handle

hrkrshnn

Vulnerability details

Consider shortening revert strings

Several revert strings in SwappableYieldSource.sol are more than 32 bytes. Try to keep everything below 32 bytes. Even better would be replacing it with Solidity's custom errors from 0.8.4.

Example:

  function transferERC20(IERC20Upgradeable erc20Token, address to, uint256 amount) external onlyOwnerOrAssetManager returns (bool) {
    require(address(erc20Token) != address(yieldSource), "SwappableYieldSource/yield-source-token-transfer-not-allowed");
    ...
}

The above string is 60 characters. Everything above 32 characters would need an additional mstore. This would incur a cost for an additional mstore, cost for memory expansion, as well as additional stack operation costs. This is only relevant when the revert condition is met.

Shortening would also reduce the deploy cost for the contract in all cases.

If possible, it's recommended to change this to:

pragma solidity ^0.8.4;

/// A long NatSpec comment explaning this in detail, without costing on chain gas.
error YieldSourceTokenTransferNotAllowed();

function transferERC20(IERC20Upgradeable erc20Token, address to, uint256 amount) external onlyOwnerOrAssetManager returns (bool) {
    if (address(erc20Token) == address(yieldSource))
        revert YieldSourceTokenTransferNotAllowed();
    ...
 }

The above change will also decrease gas.

There are several other instances where large revert strings are used. All of them can be shortened / replaced.

Handle

hrkrshnn

Vulnerability details

Use 0.8.4

The version 0.8.4 includes an important low level inliner that can save gas. Upgrading MStableYieldSource.sol from 0.8.2 to 0.8.4 should improve gas.

Handle

hrkrshnn

Vulnerability details

Replace an external call with the immutable variable

Context

@@ -59,7 +59,7 @@ contract MStableYieldSource is IYieldSource, ReentrancyGuard {

     /// @notice Approves of the max spend amount for the Savings contract.
     function approveMax() public {
-        IERC20(savings.underlying()).safeApprove(address(savings), type(uint256).max);
+        mAsset.safeApprove(address(savings), type(uint256).max);

         emit ApprovedMax(msg.sender);
     }

Instead of an expensive external call, the value would be replaced by a cheap push operation.