code-423n4 / 2023-09-asymmetry-findings

2 stars 1 forks source link

Upgraded Q -> 2 from #70 [1696570029024] #74

Closed c4-judge closed 12 months ago

c4-judge commented 12 months ago

Judge has assessed an item in Issue #70 as 2 risk. The relevant finding follows:

[L-02] depositRewards() might make the safEth:vAfEth allocation even further from ratio In depositRewards(), the ETH transferred to the contract is deposited into either safEth or vAfEth based on the following logic:

AfEth.sol#L286-L292

    uint256 totalTvl = (safEthTvl + votiumTvl);
    uint256 safEthRatio = (safEthTvl * 1e18) / totalTvl;
    if (safEthRatio < ratio) {
        ISafEth(SAF_ETH_ADDRESS).stake{value: amount}(0);
    } else {
        votiumStrategy.depositRewards{value: amount}(amount);
    }

As seen from above, it calculates the safEthRatio using safEth's TVL divided by overall TVL, and checks if safEthRatio is below ratio.

However, such an implementation could actually bring the safEth:vAfEth ratio further away from ratio if the difference between safEthRatio and ratio is small, and the amount of ETH to deposit is large.

For example, if ratio - safEthRatio = 1, the safEth:vAfEth ratio is effectively already equal to ratio. However, due to the logic above, all ETH will be deposited into safEth, which would increase safEthRatio to become much higher than ratio.

Recommendation Consider splitting the amount of ETH to deposit between safEth and vAfEth based on ratio, and deposit into both of them whenever depositRewards() is called (similar to deposit()).

This would ensure that the safEth:vAfEth ratio always averages towards ratio whenever depositRewards() is called.

c4-judge commented 12 months ago

0xleastwood marked the issue as duplicate of #48

c4-judge commented 12 months ago

0xleastwood marked the issue as satisfactory

c4-judge commented 12 months ago

This auto-generated issue was withdrawn by 0xleastwood