Asymmetry contest - Bason's results

Low and Non-Critical Issues Summary

---

[NC-01] Use latest Solidity version

Solidity pragma versioning should be upgraded to latest available version - 0.8.19

---

|[NC-02]| Use stable pragma statement

Using a floating pragma statement ^0.8.13 is discouraged as code can compile to different bytecodes with different compiler versions. Use a stable pragma statement to get a deterministic bytecode.

|[NC-03]| Use named imports instead of plain import FILE.SOL

Recommendation: `import {contract1, interface1} from "filename.sol";

|[NC-04]| Constants should be defined rather than using magic numbers

SafEth.sol minAmount = 5 * 10 ** 17; maxAmount = 200 * 10 ** 18; preDepositPrice = 10 ** 18; underlyingValue += (derivatives[i].ethPerDerivative(derivatives[i].balance()) * derivatives[i].balance()) / 10 ** 18; preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply uint derivativeReceivedEthValue = (derivative.ethPerDerivative( depositAmount ) * depositAmount) / 10 ** 18;

Reth.sol Define "RocketPool" as a private constant and return it in the method function name() public pure returns (string memory) { return "RocketPool"; }

---

uint rethPerEth = (10 ** 36) / poolPrice(); uint256 minOut = ((((rethPerEth * msg.value) / 10 ** 18) * ((10 ** 18 - maxSlippage))) / 10 ** 18); (poolPrice() * 10 ** 18) / (10 ** 18); factory.getPool(rocketTokenRETHAddress, W_ETH_ADDRESS, 500) return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);

---

WstEth.sol Define "Lido" as a private constant and return it in the method function name() public pure returns (string memory) { return "Lido"; }

return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18);

---

SfrxEth.sol

uint256 minOut = (((ethPerDerivative(_amount) * _amount) / 10 ** 18) * (10 ** 18 - maxSlippage)) / 10 ** 18;

Extract "Frax" to a private constant and return it in the method function name() public pure returns (string memory) { return "Frax"; }

[NC-05] You can use named parameters in mapping types

Even though it is out of contest scope I would suggest it:

From Solidity 0.8.18 you can use named parameters in mapping types. This will make the code much more readable. SafEthStorage.sol - for example you can use named parameters here. mapping(uint256 => IDerivative) public derivatives mapping(uint256 => uint256) public weights;

[L-01] MISSING EVENT FOR CRITICAL PARAMETER CHANGE

Emitting events allows monitoring activities with off-chain monitoring tools. I would suggest creating custom events and emitting them on critical parameter changes. I am giving an example for a few files.

Reth.sol
Create a custom event DepositSuccessful(indexed address depositor)

function deposit( uint256 rethMinted = rethBalance2 - rethBalance1; emit DepositSuccessful(rethAddress())

WstEth.sol
Create a custom event DepositSuccessful(indexed address depositor)
function deposit(
uint256 wstEthAmount = wstEthBalancePost - wstEthBalancePre;
emit DepositSuccessful(WST_ETH)

---

[L-02] POSSIBLE DIVISION BY 0 IF CONTRACTS ARE CONFIGURED IMPROPERLY

Possible division by 0 can appear contracts if they are configured improperly and cause errors. This will prevent the contract from executing. I'd recommend implementing a check for those and throwing a custom error where they are implemented, even if it is in the constructor. I am providing some examples. SafEth.sol Total weight can be 0 uint256 ethAmount = (msg.value * weight) / totalWeight; Total supply can be 0 _safEthAmount) / safEthTotalSupply;

Reth.sol
Looks like poolPrice can be 0 if configured improperly
if (!poolCanDeposit(msg.value)) {
            uint rethPerEth = (10 ** 36) / poolPrice();

|[L-03]| Possible to pass address 0 in functions that pass ownership

There are functions that accept an address as a parameter and I recommend to add a check whether someone is passing address 0. This is not critical but mistakes can happen.

Reth.sol function initialize(address _owner) external initializer { _transferOwnership(_owner); maxSlippage = (1 * 10 ** 16); // 1% }

function swapExactInputSingleHop(
        address _tokenIn,
        address _tokenOut,
        uint24 _poolFee,
        uint256 _amountIn,
        uint256 _minOut
    ) private returns (uint256 amountOut) {
        IERC20(_tokenIn).approve(UNISWAP_ROUTER, _amountIn);

SafEth.sol

function addDerivative( address _contractAddress, uint256 _weight ) external onlyOwner { derivatives[derivativeCount] = IDerivative(_contractAddress);

SfrxEth.sol

function initialize(address _owner) external initializer {
        _transferOwnership(_owner);
        maxSlippage = (1 * 10 ** 16); // 1%
    }

Gas Optimization Findings

---

[GAS-01] Use if statements instead of require and revert with custom errors instead of using strings

Even thought it is a public finding I would strongly advise to replace all require statements if statements and use custom errors. There are many places in the contract where require is being used and that can save quite a bit of gas.

SafEth.sol

require(pauseStaking == false, "staking is paused"); require(msg.value >= minAmount, "amount too low"); require(msg.value <= maxAmount, "amount too high");

require(pauseUnstaking == false, "unstaking is paused");

Reth.sol
require(sent, "Failed to send Ether");
require(rethBalance2 > rethBalance1, "No rETH was minted");

SfrxEth.sol require(sent, "Failed to send Ether");

WstEth.sol
require(sent, "Failed to send Ether");

|[GAS-02]| Make some constants private

Reth.sol All of these can be made private to save gas

address public constant ROCKET_STORAGE_ADDRESS = 0x1d8f8f00cfa6758d7bE78336684788Fb0ee0Fa46; address public constant W_ETH_ADDRESS = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; address public constant UNISWAP_ROUTER = 0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45; address public constant UNI_V3_FACTORY = 0x1F98431c8aD98523631AE4a59f267346ea31F984;

SfrxEth.sol

address public constant SFRX_ETH_ADDRESS =
    0xac3E018457B222d93114458476f3E3416Abbe38F;
address public constant FRX_ETH_ADDRESS =
    0x5E8422345238F34275888049021821E8E08CAa1f;
address public constant FRX_ETH_CRV_POOL_ADDRESS =
    0xa1F8A6807c402E4A15ef4EBa36528A3FED24E577;
address public constant FRX_ETH_MINTER_ADDRESS =
    0xbAFA44EFE7901E04E39Dad13167D089C559c1138;

WstEth.sol

address public constant WST_ETH = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0; address public constant LIDO_CRV_POOL = 0xDC24316b9AE028F1497c275EB9192a3Ea0f67022; address public constant STETH_TOKEN = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;

|[GAS-03]| Use calldata type instead of memory

SafEth.sol Here we can use calldata type instead of memory function initialize( string memory _tokenName, string memory _tokenSymbol )

|[GAS-04]| Compute off-chain and use directly the hashes of contract addresses if possible

There are multiple places in the contract where we use keccak256 to hash some ABI of the contract. If this can be done off-chain and used in the contract - we will save quite a bit of gas.

Reth.sol
We can pass the already generated hash of contract.address and the second parameter. There are 6 occurrences of this in this contract.

RocketStorageInterface(ROCKET_STORAGE_ADDRESS).getAddress(
            keccak256(
                abi.encodePacked("contract.address", "rocketTokenRETH")
            )
        );


address rocketDepositPoolAddress = RocketStorageInterface(
        ROCKET_STORAGE_ADDRESS
    ).getAddress(
        keccak256(
            abi.encodePacked("contract.address", "rocketDepositPool")
        )
    );

address rocketProtocolSettingsAddress = RocketStorageInterface(
    ROCKET_STORAGE_ADDRESS
).getAddress(
        keccak256(
            abi.encodePacked(
                "contract.address",
                "rocketDAOProtocolSettingsDeposit"
                )
            )
        );

address rocketTokenRETHAddress = RocketStorageInterface(
            ROCKET_STORAGE_ADDRESS
        ).getAddress(
                keccak256(
                    abi.encodePacked("contract.address", "rocketTokenRETH")
                )
            );

address rocketTokenRETHAddress = RocketStorageInterface(
        ROCKET_STORAGE_ADDRESS
    ).getAddress(
            keccak256(
                abi.encodePacked("contract.address", "rocketTokenRETH")
            )
        );

|[GAS-05]| Streamline if/else statements to save gas

Reth.sol
Change the order of the if check and check if amount is bigger or equal than the minimum deposit size of the pool. 
We will save calling the getBalance function and doing the addition and save gas.
BEFORE:
return
    rocketDepositPool.getBalance() + _amount <=
    rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() &&
    _amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit();

AFTER:
_amount >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit() &&
rocketDepositPool.getBalance() + _amount <=
    rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize()

You can remove the else keyword here.
BEFORE:
if (poolCanDeposit(_amount))
    return RocketTokenRETHInterface(rethAddress()).getEthValue(10 ** 18);
    else return (poolPrice() * 10 ** 18) / (10 ** 18);

AFTER:
if (poolCanDeposit(_amount))
    return RocketTokenRETHInterface(rethAddress()).getEthValue(10 ** 18);
    return (poolPrice() * 10 ** 18) / (10 ** 18);

SafEth.sol Move the if (weight == 0 ) check to be immediately after declaring the variable. You will save gas from sloading from the derivatives mapping (derivatives[i]) BEFORE: for (uint i = 0; i < derivativeCount; i++) { uint256 weight = weights[i]; IDerivative derivative = derivatives[i]; if (weight == 0) continue; uint256 ethAmount = (msg.value * weight) / totalWeight;

AFTER: for (uint i = 0; i < derivativeCount; i++) { uint256 weight = weights[i]; if (weight == 0) continue; IDerivative derivative = derivatives[i]; uint256 ethAmount = (msg.value * weight) / totalWeight;

|[GAS-06]| Assign a local variable for for loops when comparing to state variables to save from SLOAD operations

Instead of reading directly from state variables and wasting gas on SLOAD operations every time we iterate over the loop, we can assign a local variable to be the length of what we are checking against and do an SLOAD operation just once.

SafEth.sol BEFORE: for (uint i = 0; i < derivativeCount; i++) AFTER: uint derivativeCountLength = derivativeCount for (uint i = 0; i < derivativeCountLength; i++)

BEFORE: for (uint i = 0; i < derivativeCount; i++) { if (derivatives[i].balance() > 0) derivatives[i].withdraw(derivatives[i].balance()); } AFTER: uint derivativeCountLength = derivativeCount for (uint i = 0; i < derivativeCountLength; i++) { if (derivatives[i].balance() > 0) derivatives[i].withdraw(derivatives[i].balance()); }