Asymmetry contest - carlitox477's results

Lines of code

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

Vulnerability details

Description

Curve's pool price_oracle() returns an Exponential Moving Average (EMA) of the asset price queried.

According to investopedia: An EMA is a type of moving average (MA) that places a greater weight and significance on the most recent data points. Its formula can be summarize in:

$EMA_{t_{x}} = price_{t_{x}} \times \alpha + EMA_{t_{x-1}} \times (1 - \alpha)$

where:

• $EMA_{t_{x}}$ is the EMA at time $t_{x}$
• $ price_{t_{x}} $ is the price at time $t_{x}$
• $ EMA_{t_{x-1}} $ is the previous EMA, calculated at time $t_{x-1}$
• $ \alpha $ is a coefficient to indicate the weight of $EMA_{t_{x-1}}$ and $price_{t_{x}}$ in new EMA calculation.

Curve decided that the contribution of the last oracle price will decrease exponentially with time, controlled by a tunable half life parameter. This means that $\alpha = 2^{-\frac{t_{x}-t_{x-1}}{T_{1 / 2}}}$, where $T_{1 / 2}$ is the mentioned tunable half life parameter.

Curve function price_oracle() is detailed in Automatic market-making with dynamic peg (section "Algorithm for repegging"), written to explain Curve pools. The implementation for frxETH/ETH pool is defined as:

@external
@view
@nonreentrant('lock')
def price_oracle() -> uint256:
    amp: uint256 = self._A()
    xp: uint256[N_COINS] = self.balances
    D: uint256 = self._get_D(xp, amp)
    return self._ma_price(xp, amp, D)

And _ma_price is defined as:

@internal
@view
def _ma_price(xp: uint256[N_COINS], amp: uint256, D: uint256) -> uint256:
    p: uint256 = self._get_p(xp, amp, D)
    ema_mul: uint256 = self.exp(-convert((block.timestamp - self.ma_last_time) * 10**18 / self.ma_exp_time, int256))
    return (self.ma_price * ema_mul + p * (10**18 - ema_mul)) / 10**18

As we can see ma_exp_time is equivalent to $T_{ 1 / 2}$. For the pool contract, its value is 10387 seconds (2 hours, 53.11 minutes). A price oracle update is trigger whenever a trade, unbalanced withdraw or deposit occur. As a consequences, if these events does not occur for a long time, then function price_oracle would weight the oldest value greater than the new one, also if the new price informed is to big then the price will be manipulable.

Impact

Curve invariant tends to be stable as long as the imbalance in the pool is not too huge. Taking into account that these means that there is a risk that the price of the pool can be manipulated, then checking the freshness and deviation of the oracle is essential to guarantee that, in this scenario, this can not be used to manipulate function ethPerDerivative outcome.

POC

Reacted Cartel (2022-02) M-17

Mitigation steps

1. Ensure the price was updated within a certain limit
2. Check that the last reported price pLast has not deviated too far from the current oracle price p_old

Lines of code

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

Vulnerability details

Description

Current implementation of SafEth.stake allows to freeze ETH if derivativeCount == 0. This happens because the for loops will be ignored, minting 0 SafETH to the function caller. Function that enable unstake or rebalance will not be able to manipulate the staked ETH.

Impact

If derivativeCount == 0, any user who calls SafEth.stake with ETH will freeze the amount sent in the contract forever.

POC

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

        uint256 underlyingValue = 0;

        // @audit if derivativeCount = 0 for loop is ignored, then underlyingValue = 0
        // Getting underlying value in terms of ETH for each derivative
        for (uint i = 0; i < derivativeCount; i++)
            underlyingValue +=
                (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;

        uint256 totalSupply = totalSupply();
        uint256 preDepositPrice; // Price of safETH in regards to ETH
        if (totalSupply == 0)
            preDepositPrice = 10 ** 18; // initializes with a price of 1
        else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

        uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
        // @audit if derivativeCount = 0 for loop is ignored, then totalStakeValueEth = 0
        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;

            // This is slightly less than ethAmount because slippage
            uint256 depositAmount = derivative.deposit{value: ethAmount}();
            uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
                depositAmount
            ) * depositAmount) / 10 ** 18;
            totalStakeValueEth += derivativeReceivedEthValue;
        }
        // mintAmount represents a percentage of the total assets in the system

        // @audit Given totalStakeValueEth = 0, then mintAmount = 0
        uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;

        // @audit mint 0 tokens
        _mint(msg.sender, mintAmount);
        emit Staked(msg.sender, msg.value, mintAmount);
    }

Given that ERC20._mint(msg.sender,mintAmount) does not revert if mintAmount = 0, then funds will be lost forever (unstake and rebalanceToWeights use the difference between the balance at the start and the balance in the middle of their execution to do calculations, not even the owner will be able to get the ETH)

Mitigation steps

Check that derivativeCount != 0 at the beginning of the function.

wstETH: If the protocol is meant to be multichain, then WST_ETH, LIDO_CRV_POOL and STETH_TOKEN should be immutable and the constructor modified

In order to not change the current code every time a deployment is done, the mentioned variables should be delcared as immutable and their values set in the constructor:

    contract WstEth is IDerivative, Initializable, OwnableUpgradeable {
-      address public constant WST_ETH =
-        0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0;
+      address public immutable WST_ETH;
-      address public constant LIDO_CRV_POOL =
-          0xDC24316b9AE028F1497c275EB9192a3Ea0f67022;
+      address public immutable LIDO_CRV_POOL;
-       address public constant STETH_TOKEN =
-          0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;
+       address public immutable STETH_TOKEN;

        uint256 public maxSlippage;

        // As recommended by https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable
        /// @custom:oz-upgrades-unsafe-allow constructor
-       constructor() {
+       constructor(address _WST_ETH, address _LIDO_CRV_POOL, address _STETH_TOKEN) {
            _disableInitializers();
+           WST_ETH = _WST_ETH;
+           LIDO_CRV_POOL = _LIDO_CRV_POOL;
+           STETH_TOKEN = _STETH_TOKEN;
        }

WstEth.setMaxSlippage(uint256), Reth.setMaxSlippage(uint256) and Sfrx.setMaxSlippage(uint256) should limit their inputs values to 1e18

Given that 1e16 is 1%, then 100% would be 1e18. Given that the slippage should never be greater than 100%.

This would DOS function withdraw/deposit in the different contracts:

• wstEth
• Reth
• SfrxEth

This constraint must be checked.

SafETH.stake(): avoid totalSupply shadowing

Current code creates a variable which shadows totalSupply. It's name should be replaced by _totalSupply. This means:

-   uint256 totalSupply = totalSupply();
+   uint256 _totalSupply = totalSupply();
    uint256 preDepositPrice; // Price of safETH in regards to ETH
-   if (totalSupply == 0)
+   if (_totalSupply == 0)
        preDepositPrice = 10 ** 18; // initializes with a price of 1
-   else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;
+   else preDepositPrice = (10 ** 18 * underlyingValue) / _totalSupply;

SafETH Stuck ETH would be freeze forever

Current SafETH allows receiving ETH from anyone. In case that a user/contract mistakenly sent ETH to this SafEth or in case that msg.value * weight < totalWeight in stake function, these funds will be freeze forever. Given that rebalance function takes into account the funds at the start of it calls, and the funds after converting derivatives to ETH.

There are three ways to approach this issue:

Option A: Stuck ETH belongs to the owner

This would mean that a withdrawStuckETH function should be added. Given that ETH corresponding to stake/unstake actions are immediately sent to the corresponding contract/user, this function is easy to implement

    function withdrawStuckETH() external onlyOwner(){
        address(msg.sender).call{value: address(this).balance}("");
    }

Option B: Stuck ETH belongs to the first one who realize about it

This would mean adding the same function written before without the onlyOwner modifier

Option C: Consider stuck ETH when rebalanceToWeights is called

This would mean that rebalanceToWeights should be changed to:

    function rebalanceToWeights() external onlyOwner {
-       uint256 ethAmountBefore = address(this).balance;
        for (uint i = 0; i < derivativeCount; i++) {
            if (derivatives[i].balance() > 0)
                derivatives[i].withdraw(derivatives[i].balance());
        }
-       uint256 ethAmountAfter = address(this).balance;
-       uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;
+       uint256 ethAmountToRebalance = address(this).balance;

        for (uint i = 0; i < derivativeCount; i++) {
            if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
            uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
                totalWeight;
            // Price will change due to slippage
            derivatives[i].deposit{value: ethAmount}();
        }
        emit Rebalanced();
    }

It would also be advisable to remove onlyOwner modifier to allow anyone to call this function, given that there is no way for the caller to do any kind of manipulation of state variables.

SafEth.addDerivative(address,uint256) and SafEth.adjustWeight(address,uint256) should call SafEth.rebalanceToWeights() in order to keep the desire balance

When SafEth.addDerivative(address,uint256) and SafEth.adjustWeight(address,uint256) are called, the intention is to modify the current distribution of ETH staked. In order to make the changes in weights effective, function SafEth.rebalanceToWeights should be called. To do this:

1. SafEth.rebalanceToWeights() visibility should be change to public
2. SafEth.rebalanceToWeights() should be called at the end of SafEth.addDerivative(address,uint256) and SafEth.adjustWeight(address,uint256)

SafEth.adjustWeight(address,uint256) can be involuntary DOSed if enough derivatives are added

Given that to calculate the total weight a for loop is used which require access to weights mapping, this function can be DOSed if enough derivatives are added. To avoid this current function can be replaced for:

    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
+       uint256 previousWeight = weights[_derivativeIndex]
        weights[_derivativeIndex] = _weight;
-       uint256 localTotalWeight = 0;
-       for (uint256 i = 0; i < derivativeCount; i++)
-           localTotalWeight += weights[i];
-       totalWeight = localTotalWeight;
+       totalWeight = totalWeight + _weight - previousWeight;
        emit WeightChange(_derivativeIndex, _weight);
    }

SafEth.addDerivative(address,uint256) can be involuntary DOSed if enough derivatives are added

Given that to calculate the total weight a for loop is used which require access to weights mapping, this function can be DOSed if enough derivatives are added. To avoid this current function can be replaced for:

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

-       uint256 localTotalWeight = 0;
-       for (uint256 i = 0; i < derivativeCount; i++)
-           localTotalWeight += weights[i];
-       totalWeight = localTotalWeight;
+       totalWeight += _weight;
        emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
    }

SafEth.setMinAmount(uint256) should check that _minAmount != minAmount && _minAmount <= maxAmount

In order to emit a meaningful event it should be checked that _minAmount != minAmount

In order to avoid DOS stake function, then _minAmount <= maxAmount should be checked. This means:

    function setMinAmount(uint256 _minAmount) external onlyOwner {
+       require(_minAmount != minAmount,ERROR_NOT_STATE_CHANGE);
+       require(_minAmount <= maxAmount, ERROR_MIN_AMOUNT_SHOULD_BE_LEQ_THAN_MAX_AMOUNT);
        minAmount = _minAmount;
        emit ChangeMinAmount(minAmount);
    }

SafEth.setMaxAmount(uint256) should check that _maxAmount != maxAmount && minAmount <= _maxAmount

In order to emit a meaningful event it should be checked that _maxAmount != maxAmount

In order to avoid DOS stake function, then minAmount <= _maxAmount should be checked. This means:

    function setMaxAmount(uint256 _maxAmount) external onlyOwner {
+       require(_maxAmount != maxAmount,ERROR_NOT_STATE_CHANGE);
+       require(minAmount <= _maxAmount, ERROR_MAX_AMOUNT_SHOULD_BE_GEQ_THAN_MIN_AMOUNT);
        maxAmount = _maxAmount;
        emit ChangeMaxAmount(maxAmount);
    }

SafEth.setPauseStaking should check that current pauseStaking value is really being change

Otherwise, current implementation would allow emitting meaningless events

function setPauseStaking(bool _pause) external onlyOwner {
        pauseStaking = _pause;
+       require(pauseStaking != _pause, ERROR_NOT_STATE_CHANGE);
        emit StakingPaused(pauseStaking);
    }

SafEth.setPauseUnstaking should check that current pauseUnstaking value is really being change

Otherwise, current implementation would allow emitting meaningless events

    function setPauseUnstaking(bool _pause) external onlyOwner {
+       require(pauseUnstaking != _pause, ERROR_NOT_STATE_CHANGE);
        pauseUnstaking = _pause;
        emit UnstakingPaused(pauseUnstaking);
    }

SafEth.stake() can use derivatives and weights storage pointer to save gas in both for loop

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

        uint256 underlyingValue = 0;
+       mapping(uint256 => IDerivative) storage _derivatives = derivatives;
+       IDerivative _derivative;
        // Getting underlying value in terms of ETH for each derivative
        for (uint i = 0; i < derivativeCount; i++)
            underlyingValue +=
-               (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
-                   derivatives[i].balance()) /
+               _derivative = _derivatives[i];
+               (_derivative.ethPerDerivative(_derivative.balance()) *
+                   _derivative.balance()) /
                10 ** 18;

        uint256 totalSupply = totalSupply();
        uint256 preDepositPrice; // Price of safETH in regards to ETH
        if (totalSupply == 0)
            preDepositPrice = 10 ** 18; // initializes with a price of 1
        else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

        uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
+       mapping(uint256 => uint256) storage _weights = weights;
        for (uint i = 0; i < derivativeCount; i++) {
-           uint256 weight = weights[i];
+           uint256 weight = _weights[i];
-           IDerivative derivative = derivatives[i];
+           _derivative = _derivatives[i];
            if (weight == 0) continue;
            uint256 ethAmount = (msg.value * weight) / totalWeight;

            // This is slightly less than ethAmount because slippage
            uint256 depositAmount = derivative.deposit{value: ethAmount}();
            uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
                depositAmount
            ) * depositAmount) / 10 ** 18;
            totalStakeValueEth += derivativeReceivedEthValue;
        }
        // mintAmount represents a percentage of the total assets in the system
        uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
        _mint(msg.sender, mintAmount);
        emit Staked(msg.sender, msg.value, mintAmount);
    }

SafEth.stake() can cache derivativeCount and totalWeight to save gas

This will reduce storage access in both for loops in each iteration

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

        uint256 underlyingValue = 0;

        // Getting underlying value in terms of ETH for each derivative
+       uint256 _derivativeCount = derivativeCount;
-       for (uint i = 0; i < derivativeCount; i++)
+       for (uint i; i < _derivativeCount; unchecked{++i})
            underlyingValue +=
                (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;

        uint256 totalSupply = totalSupply();
        uint256 preDepositPrice; // Price of safETH in regards to ETH
        if (totalSupply == 0)
            preDepositPrice = 10 ** 18; // initializes with a price of 1
        else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

        uint256 totalStakeValueEth = 0; // total amount of derivatives worth of ETH in system
+       uint256 _totalWeight = totalWeight;
-       for (uint i = 0; i < derivativeCount; i++) {
+       for (uint i; i < _derivativeCount; unchecked{++i}) {
            uint256 weight = weights[i];
            IDerivative derivative = derivatives[i];
            if (weight == 0) continue;
-           uint256 ethAmount = (msg.value * weight) / totalWeight;
+           uint256 ethAmount = (msg.value * weight) / _totalWeight;

            // This is slightly less than ethAmount because slippage
            uint256 depositAmount = derivative.deposit{value: ethAmount}();
            uint derivativeReceivedEthValue = (derivative.ethPerDerivative(
                depositAmount
            ) * depositAmount) / 10 ** 18;
            totalStakeValueEth += derivativeReceivedEthValue;
        }
        // mintAmount represents a percentage of the total assets in the system
        uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice;
        _mint(msg.sender, mintAmount);
        emit Staked(msg.sender, msg.value, mintAmount);
    }

SafEth.stake() can cache derivatives[i].balance()

In the first for loop, 2 external calls can be reduced to one by saving the queried value:

-   for (uint i = 0; i < derivativeCount; i++)
+   uint256 _derivative_balance;
+   for (uint i = 0; i < derivativeCount; i++){
+       uint256 _derivative_balance = derivatives[i].balance();
        underlyingValue +=
-           (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
-               derivatives[i].balance()) /
+            (derivatives[i].ethPerDerivative(_derivative_balance) *
+               _derivative_balance) /
            10 ** 18;
    }
    //..

SafEth.rebalanceToWeights() can cache derivatives[i].balance():

+   uint256 derivatives_balance;
    for (uint i = 0; i < derivativeCount; i++) {
+       derivatives_balance = derivatives[i].balance();
-       if (derivatives[i].balance() > 0)
-           derivatives[i].withdraw(derivatives[i].balance());
+       if (derivatives_balance > 0)
+           derivatives[i].withdraw(derivatives_balance);
    }
    //...

SafEth.rebalanceToWeights() can use derivatives and weights storage pointer to save gas in both for loops:

    function rebalanceToWeights() external onlyOwner {
        uint256 ethAmountBefore = address(this).balance;
+       mapping(uint256 => IDerivative) storage _derivatives = derivatives;
        for (uint i = 0; i < derivativeCount; i++) {
-           if (derivatives[i].balance() > 0)
-               derivatives[i].withdraw(derivatives[i].balance());
+           if (_derivatives[i].balance() > 0)
+               _derivatives[i].withdraw(_derivatives[i].balance());
        }
        uint256 ethAmountAfter = address(this).balance;
        uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;

+       mapping(uint256 => uint256) storage _weights = weights;
        for (uint i = 0; i < derivativeCount; i++) {
-           if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
+           if (_weights[i] == 0 || ethAmountToRebalance == 0) continue;
-           uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
+           uint256 ethAmount = (ethAmountToRebalance * _weights[i]) /
                totalWeight;
            // Price will change due to slippage
-           derivatives[i].deposit{value: ethAmount}();
+           _derivatives[i].deposit{value: ethAmount}();
        }
        emit Rebalanced();
    }

SafEth.rebalanceToWeights() can cache derivativeCount and totalWeight to save gas

This will avoid an storage access in each iteration of both for loops:

    function rebalanceToWeights() external onlyOwner {
        uint256 ethAmountBefore = address(this).balance;
+       uint256 _derivativeCount = derivativeCount;
-       for (uint i = 0; i < derivativeCount; i++) {
+       for (uint i; i < _derivativeCount; unchecked{++i}) {
            if (derivatives[i].balance() > 0)
                derivatives[i].withdraw(derivatives[i].balance());
        }
        uint256 ethAmountAfter = address(this).balance;
        uint256 ethAmountToRebalance = ethAmountAfter - ethAmountBefore;

+       uint256 _totalWeight = totalWeight;
-       for (uint i = 0; i < derivativeCount; i++) {
+       for (uint i; i < _derivativeCount; unchecked{++i}) {
            if (weights[i] == 0 || ethAmountToRebalance == 0) continue;
            uint256 ethAmount = (ethAmountToRebalance * weights[i]) /
-               totalWeight;
+               _totalWeight;
            // Price will change due to slippage
            derivatives[i].deposit{value: ethAmount}();
        }
        emit Rebalanced();
    }

SafEth.adjustWeight(address,uint256): Refactor function to avoid for loop and save gas

Current function make use of a for loop to recalculate totalWeight, however this action can be done by just adding input _weight to current totalWeight and substracting previous weight corresponding to _derivativeIndex. This means:

    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
+       uint previousWeight = weights[_derivativeIndex]
        weights[_derivativeIndex] = _weight;
-       uint256 localTotalWeight = 0;
-       for (uint256 i = 0; i < derivativeCount; i++)
-           localTotalWeight += weights[i];
-       totalWeight = localTotalWeight;
+       totalWeight = totalWeight - previousWeight + _weight;
        emit WeightChange(_derivativeIndex, _weight);
    }

SafEth.adjustWeight(address,uint256): Use storage pointer for weights in order to save gas

    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
+       mapping(uint256 => uint256) storage _weights = weights;
-       weights[_derivativeIndex] = _weight;
+       _weights[_derivativeIndex] = _weight;
        uint256 localTotalWeight = 0;
        for (uint256 i = 0; i < derivativeCount; i++)
-           localTotalWeight += weights[i];
+           localTotalWeight += _weights[i];
        totalWeight = localTotalWeight;
        emit WeightChange(_derivativeIndex, _weight);
    }

SafEth.adjustWeight(address,uint256): Cache derivativeCount in order to save gas

derivativeCount is accessed multiple times in for loop check, it can be cache in order to avoid its storage access

    function adjustWeight(
        uint256 _derivativeIndex,
        uint256 _weight
    ) external onlyOwner {
        weights[_derivativeIndex] = _weight;
        uint256 localTotalWeight = 0;
+       uint256 _derivativeCount = derivativeCount;
-       for (uint256 i = 0; i < derivativeCount; i++)
+       for (uint256 i = 0; i < _derivativeCount; i++)
            localTotalWeight += weights[i];
        totalWeight = localTotalWeight;
        emit WeightChange(_derivativeIndex, _weight);
    }

SafEth.addDerivative(address,uint256): Refactor function to avoid for loop and save gas

Current function make use of a for loop to recalculate totalWeight, however this action can be done by just adding input _weight to current totalWeight. This means:

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

-       uint256 localTotalWeight = 0;
-       for (uint256 i = 0; i < derivativeCount; i++)
-           localTotalWeight += weights[i];
-       totalWeight = localTotalWeight;
+       totalWeight += _weight;
        emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
    }

SafEth.addDerivative(address,uint256): Use storage pointer for weights in order to save gas

    function addDerivative(
        address _contractAddress,
        uint256 _weight
    ) external onlyOwner {
        derivatives[derivativeCount] = IDerivative(_contractAddress);
+       mapping(uint256 => uint256) storage _weights = weights;
-       weights[derivativeCount] = _weight;
+       _weights[derivativeCount] = _weight;
        derivativeCount++;

        uint256 localTotalWeight = 0;

        for (uint256 i = 0; i < derivativeCount; i++)
-           localTotalWeight += weights[i];
+           localTotalWeight += _weight[i];
        totalWeight = localTotalWeight;
        emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
    }

SafEth.addDerivative(address,uint256): Cache derivativeCount in order to save gas

derivativeCount is accessed multiple times, it can be cache in order to avoid some storage access

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

        uint256 localTotalWeight = 0;
-       for (uint256 i = 0; i < derivativeCount; i++)
+       for (uint256 i; i < _derivativeCount; unchecked{++i})
            localTotalWeight += weights[i];
        totalWeight = localTotalWeight;
-       emit DerivativeAdded(_contractAddress, _weight, derivativeCount);
+       emit DerivativeAdded(_contractAddress, _weight, _derivativeCount);
    }

SafEth.setMinAmount(uint256): Use _minAmount instead of minAmount to avoid storage access

This would avoid 1 storage access

function setMinAmount(uint256 _minAmount) external onlyOwner {
        minAmount = _minAmount;
-       emit ChangeMinAmount(minAmount);
+       emit ChangeMinAmount(_minAmount);
    }

SafEth.setMaxAmount(uint256): Use _maxAmount instead of maxAmount to avoid storage access

This would avoid 1 storage access

    function setMaxAmount(uint256 _maxAmount) external onlyOwner {
        maxAmount = _maxAmount;
-       emit ChangeMaxAmount(maxAmount);
+       emit ChangeMaxAmount(_maxAmount);
    }

SafEth.setPauseStaking(bool): Use _pause instead of pauseStaking to avoid storage access

This would avoid 1 storage access

    function setPauseStaking(bool _pause) external onlyOwner {
        pauseStaking = _pause;
-       emit StakingPaused(pauseStaking);
+       emit StakingPaused(_pause);
    }

SafEth.setPauseUnstaking(bool): Use _pause instead of pauseUnstaking to avoid storage access

This would avoid 1 storage access

    function setPauseUnstaking(bool _pause) external onlyOwner {
        pauseUnstaking = _pause;
-       emit UnstakingPaused(pauseUnstaking);
+       emit UnstakingPaused(_pause);
    }