Asymmetry contest - pixpi's results

---

<a name="QA-1">[QA-1]</a> Remove unused imports

There are several instances of unused import statements. Removing them will make the code cleaner and reduce deployment costs.

6 instances - 2 files

File: contracts/SafEth/SafEth.sol
  4: import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
  5: import "../interfaces/IWETH.sol";
  6: import "../interfaces/uniswap/ISwapRouter.sol";
  7: import "../interfaces/lido/IWStETH.sol";
  8: import "../interfaces/lido/IstETH.sol";

File: contracts/SafEth/derivatives/Reth.sol
  5: import "../../interfaces/frax/IsFrxEth.sol";

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L4-L8

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L5

---

<a name="QA-2">[QA-2]</a> Specify a fixed pragma version

By locking the pragma version, developers can ensure that their code will work as expected with a known version of the Solidity compiler. This can make it easier to maintain and upgrade Solidity code over time, as new versions of the language are released.

https://consensys.github.io/smart-contract-best-practices/development-recommendations/solidity-specific/locking-pragmas/

5 instances - 5 files

File: contracts/SafEth/SafEth.sol
  2: pragma solidity ^0.8.13;

File: contracts/SafEth/SafEthStorage.sol
  2: pragma solidity ^0.8.13;

File: contracts/SafEth/derivatives/Reth.sol
  2: pragma solidity ^0.8.13;

File: contracts/SafEth/derivatives/SfrxEth.sol
  2: pragma solidity ^0.8.13;

File: contracts/SafEth/derivatives/WstEth.sol
  2: pragma solidity ^0.8.13;

Recommendation code example:

File: contracts/SafEth/SafEth.sol
-  2: pragma solidity ^0.8.13;

+  2: pragma solidity 0.8.13;

---

<a name="QA-3">[QA-3]</a> Inconsistent use of uint / uint256 type

Although uint is alias for uint256 type, using both of them in the project makes code less readable.

Therefore, it is advisable to stick with just one, either uint or uint256.

16 instances - 2 files

File: contracts/SafEth/SafEth.sol
  26:    event Staked(address indexed recipient, uint ethIn, uint safEthOut);

  27:    event Unstaked(address indexed recipient, uint ethOut, uint safEthIn);

  28:    event WeightChange(uint indexed index, uint weight);

  31:        uint weight,

  32:        uint index

  71:        for (uint i = 0; i < derivativeCount; i++)

  84:        for (uint i = 0; i < derivativeCount; i++) {

  92:            uint derivativeReceivedEthValue = (derivative.ethPerDerivative(

  140:        for (uint i = 0; i < derivativeCount; i++) {

  147:        for (uint i = 0; i < derivativeCount; i++) {

  203:        uint _derivativeIndex,

  204:        uint _slippage

File: contracts/SafEth/derivatives/Reth.sol
  171:            uint rethPerEth = (10 ** 36) / poolPrice();

Recommendation:

Replace all uint instances with uint256.

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L26

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L171

---

<a name="QA-4">[QA-4]</a> Avoid using spot price

In the current implementation of the derivative contract for rETH, the deposit() function uses the spot price from Uniswap to calculate the amount of rETH to deposit if poolCanDeposit(msg.value) == false.

This introduces some possible risks, as flash loans can be used to manipulate the spot price and drain the smart contract's funds.

Although no critical issues were identified by manipulating the spot price in the current version of the smart contracts, there is a risk that it could be used as an attack vector in future versions.

To mitigate these risks, one possible solution is to use the time-weighted average price (TWAP) from Uniswap instead.

1 instance - 1 file

File: contracts/SafEth/derivatives/Reth.sol
  237:        IUniswapV3Pool pool = IUniswapV3Pool(
  238:            factory.getPool(rocketTokenRETHAddress, W_ETH_ADDRESS, 500)
  239:        );
  240:        (uint160 sqrtPriceX96, , , , , , ) = pool.slot0();
  241:        return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L237-L241

---

<a name="QA-5">[QA-5]</a> Missing NatSpec @return comments

The provided functions below lack NatSpec @return comments, and it is recommended to enhance their documentation by adding them.

16 instances - 3 files

File: contracts/SafEth/derivatives/Reth.sol
  50:    function name() public pure returns (string memory) {

  66:    function rethAddress() private view returns (address) {

  89:    ) private returns (uint256 amountOut) {

  120:   function poolCanDeposit(uint256 _amount) private view returns (bool) {

  156:   function deposit() external payable onlyOwner returns (uint256) {

  211:   function ethPerDerivative(uint256 _amount) public view returns (uint256) {

  221:   function balance() public view returns (uint256) {

  228:   function poolPrice() private view returns (uint256) {

File: contracts/SafEth/derivatives/SfrxEth.sol
  44:    function name() public pure returns (string memory) {

  94:    function deposit() external payable onlyOwner returns (uint256) {

  111:   function ethPerDerivative(uint256 _amount) public view returns (uint256) {

  122:   function balance() public view returns (uint256) {

File: contracts/SafEth/derivatives/WstEth.sol
  41:    function name() public pure returns (string memory) {

  73:    function deposit() external payable onlyOwner returns (uint256) {

  86:    function ethPerDerivative(uint256 _amount) public view returns (uint256) {

  93:    function balance() public view returns (uint256) {

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L50

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/SfrxEth.sol#L44

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/WstEth.sol#L41

---

<a name="QA-6">[QA-6]</a> Missing NatSpec @param comments

The provided functions below lack NatSpec @param comments, and it is recommended to enhance their documentation by adding them.

6 instances - 3 files

File: contracts/SafEth/derivatives/Reth.sol
  107:    function withdraw(uint256 amount) external onlyOwner {

File: contracts/SafEth/derivatives/SfrxEth.sol
  51:    function setMaxSlippage(uint256 _slippage) external onlyOwner {

  111:   function ethPerDerivative(uint256 _amount) public view returns (uint256) {

File: contracts/SafEth/derivatives/WstEth.sol
  48:    function setMaxSlippage(uint256 _slippage) external onlyOwner {

  56:    function withdraw(uint256 _amount) external onlyOwner {

  86:    function ethPerDerivative(uint256 _amount) public view returns (uint256) {

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L107

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/SfrxEth.sol#L51

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/WstEth.sol#L48

---

Fix imports

---

<a name="QA-7">[QA-7]</a> Unused function parameter _amount

It is recommended to comment out unused parameters in function declarations. That improves code readability and helps to avoid possible mistakes in the future.

2 instances - 2 files

File: contracts/SafEth/derivatives/SfrxEth.sol
  111:   function ethPerDerivative(uint256 _amount) public view returns (uint256) {

File: contracts/SafEth/derivatives/WstEth.sol
  86:    function ethPerDerivative(uint256 _amount) public view returns (uint256) {

Recommendation code example:

File: contracts/SafEth/derivatives/SfrxEth.sol
-  111:   function ethPerDerivative(uint256 _amount) public view returns (uint256) {

+  111:   function ethPerDerivative(uint256 /*_amount*/) public view returns (uint256) {

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/SfrxEth.sol#L111

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/WstEth.sol#L86

---

Gas Optimizations

---

In the following report gas optimizations are sorted by the estimated amount of gas savings.

All gas estimations were calculated by using Remix IDE and deploying two separate contracts that differ only in one small part which was being tested.

Settings used: {version: "0.8.13", settings: {optimizer: {enabled: true, runs: 100000}

---

<a name="GAS-1"></a>[GAS-1] Refactor weights to be normalized

Current implementation uses totalWeight to keep track of the total sum of all weights.

However, totalWeight is updated only in addDerivative() & adjustWeight() functions, but used inside the stake() function's for loop to calculate normalized weights.

It's safe to assume that function stake() will be used much more frequently than addDerivative() & adjustWeight().

Therefore, the code could be refactored in a way that saves gas by calculating and saving normalized values for weights in addDerivative() & adjustWeight() functions.

This allows to remove totalWeight state variable which will save gas both during deployment and code execution.

Estimated gas savings for one stake() function call: 2163 gas + 163 gas * (number of non-zero weights - 1)

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L187-L193

Recommendation code:

File: contracts/SafEth/SafEth.sol
+       unit256 private constant WEIGHT_BASE = 10000; // choose presicion

- 88:            uint256 ethAmount = (msg.value * weight) / totalWeight;
+ 88:            uint256 ethAmount = (msg.value * weight) / WEIGHT_BASE;

- 149:           uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
- 150:               totalWeight;
+ 149:           uint256 ethAmount = (ethAmountToRebalance * weights[i]) / WEIGHT_BASE;

// refactored addDerivative()
- 187:        weights[derivativeCount] = _weight;
- 188:        derivativeCount++;
- 189:
- 190:        uint256 localTotalWeight = 0;
- 191:        for (uint256 i = 0; i < derivativeCount; i++)
- 192:            localTotalWeight += weights[i];
- 193:        totalWeight = localTotalWeight;

+ 187:        addWeight(_weight);
+             unchecked {
+                 derivativeCount++;
+             }

// refactored adjustWeight()
    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) public onlyOwner {
        require(_weight <= WEIGHT_BASE, "Wrong weight");
        // limit weight to be at least 1%
        require(_weight >= WEIGHT_BASE / 100 || _weight == 0, "Wrong weight");
        require(derivativeCount > 0, "No derivatives created");
        require(_derivativeIndex < derivativeCount, "Wrong derivative index");

        if (!_checkNonZeroSum()) {
            require(_weight == WEIGHT_BASE, "Wrong weight, zero sum");
        }

        uint256 weightsLen = derivativeCount;
        uint256 oldWeight = weights[_derivativeIndex];
        uint256 remainder = WEIGHT_BASE - _weight;

        for (uint256 i = 0; i < weightsLen; ) {
            if (i != _derivativeIndex) {
                unchecked {
                    weights[i] = weights[i] * (WEIGHT_BASE - _weight) / (WEIGHT_BASE - oldWeight);
                    remainder -= weights[i];
                    i++;
                }
            } else {
               weights[_derivativeIndex] = _weight;
               unchecked {
                    i++;
                }
            }
        }

        // assigning remainder to any non-zero weight
        // in order to keep WEIGHT_BASE invariant
        for (uint256 i = 0; i < weightsLen;) {
            if (weights[i] != 0) {
                weights[i] += remainder;
                break;
            } else {
                unchecked {
                    i++;
                }
            }
        }
        emit WeightChange(_derivativeIndex, _weight);
    }

// helper functions
    // check if all current weights are not set to 0
    function _checkNonZeroSum() internal view returns (bool nonZeroSum) {
        uint256 weightsLen = derivativeCount;

        for (uint256 i = 0; i < weightsLen;) {
            if (weights[i] > 0 ) {
                nonZeroSum = true;
                break;
            }
            unchecked {
                i++;
            }
        }
    return nonZeroSum;
    }

    function addWeight(uint256 newWeight) internal {
        require(newWeight <= WEIGHT_BASE, "Wrong weight");
        // limit weight to be at least 1%
        require(newWeight >= WEIGHT_BASE / 100, "Wrong weight");

        // first weight can only be equal to WEIGHT_BASE
        if (derivativeCount == 0) {
            require(newWeight == WEIGHT_BASE, "Wrong first weight");
            weights[0] = newWeight;
        } else {
            if (!_checkNonZeroSum()) {
                require(newWeight == WEIGHT_BASE, "Wrong weight, zero sum");
                weights[derivativeCount] = newWeight;
            } else {
                uint256 weightsLen = derivativeCount;
                uint256 remainder = WEIGHT_BASE - newWeight;

                for (uint256 i = 0; i < weightsLen;) {
                    unchecked {
                        weights[i] = weights[i] * (WEIGHT_BASE - newWeight) / WEIGHT_BASE;
                        remainder -= weights[i];
                        i++;
                    }
                }

                weights[derivativeCount] = newWeight + remainder;
            }
        }
    }

---

<a name="GAS-2"></a>[GAS-2] Cache derivativeCount in memory before using inside for loop

Every conditional check i < derivativeCount inside for loop costs 100 gas due to warm access to the state variable.

Therefore, it is recommended to cache derivativeCount in memory before using in the loop.

Total gas savings: 100 gas * derivativeCount * 7

7 instances in 1 file:

File: contracts/SafEth/SafEth.sol:
  71:        for (uint i = 0; i < derivativeCount; i++)

  84:        for (uint i = 0; i < derivativeCount; i++) {

  113:        for (uint256 i = 0; i < derivativeCount; i++) {

  140:        for (uint i = 0; i < derivativeCount; i++) {

  147:        for (uint i = 0; i < derivativeCount; i++) {

  171:        for (uint256 i = 0; i < derivativeCount; i++)

  191:        for (uint256 i = 0; i < derivativeCount; i++)

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L71

Recommendation code example:

File: contracts/SafEth/SafEth.sol:
- 71:        for (uint i = 0; i < derivativeCount; i++)

+ 71:        uint256 memoryCount = derivativeCount;
+            for (uint i = 0; i < memoryCount; i++)

---

<a name="GAS-3"></a>[GAS-3] Cache calls to derivatives[i].balance() in memory before using inside for loop

There are two derivatives[i].balance() calls in every loop iteration. Caching them in memory will save gas.

Gas savings estimate: 480 gas * (number of derivatives with positive balance)

1 instance in 1 file:

File: contracts/SafEth/SafEth.sol:
  141:            if (derivatives[i].balance() > 0)
  142:                derivatives[i].withdraw(derivatives[i].balance());

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L141-L142

Recommendation code:

File: contracts/SafEth/SafEth.sol:
- 141:            if (derivatives[i].balance() > 0)
- 142:                derivatives[i].withdraw(derivatives[i].balance());

+                 uint256 derivativeBalance = derivatives[i].balance();
+ 141:            if (derivativeBalance > 0)
+ 142:                derivatives[i].withdraw(derivativeBalance);

---

<a name="GAS-4"></a>[GAS-4] Cache totalWeight in memory before using inside for loop

Every access to totalWeight variable inside for loop costs 100 gas (warm access to the state variable).

Therefore, it is recommended to cache it in memory before calling.

Total gas savings: 100 gas * derivativeCount * 2

2 instances in 1 file:

File: contracts/SafEth/SafEth.sol:
  88:            uint256 ethAmount = (msg.value * weight) / totalWeight;

  150:                totalWeight;

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L88

Recommendation code example:

File: contracts/SafEth/SafEth.sol:
+            uint256 totalWeightCache = totalWeight;
  84:        for (uint i = 0; i < derivativeCount; i++) {

- 88:            uint256 ethAmount = (msg.value * weight) / totalWeight;

+ 88:            uint256 ethAmount = (msg.value * weight) / totalWeightCache;

---

<a name="GAS-5"></a>[GAS-5] Refactor conditional checks inside for loop

Due to the fact that ethAmountToRebalance doesn't change inside for loop, conditional check ethAmountToRebalance == 0 in every iteration just wastes gas.

Therefore, it is suggested to refactor the code and move that conditional check outside for loop.

Gas savings estimate:

• if ethAmountToRebalance != 0 : 25 gas * derivativeCount
• if ethAmountToRebalance == 0 : 2400 gas * derivativeCount

1 instance in 1 file:

File: contracts/SafEth/SafEth.sol:
  147:        for (uint i = 0; i < derivativeCount; i++) {
  148:            if (weights[i] == 0 || ethAmountToRebalance == 0) continue;

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L147-L148

Recommendation code:

File: contracts/SafEth/SafEth.sol:
+            if (ethAmountToRebalance != 0)
  147:           for (uint i = 0; i < derivativeCount; i++) {
- 148:               if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
+ 148:               if (weights[i] == 0 ) continue;

---

<a name="GAS-6"></a>[GAS-6] Use unchecked for derivativeCount increment

It is safe to assume that derivativeCount will never reach the maximum of uint256 type.

Thus, it is possible to save gas by disabling overflow/underflow checks.

Gas savings: 83 gas

1 instance in 1 file:

File: contracts/SafEth/SafEth.sol:
  188:        derivativeCount++;

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L188

Recommendation code:

File: contracts/SafEth/SafEth.sol:
- 188:        derivativeCount++;

+ 188:        unchecked {
+                 derivativeCount++;
+              }

---