Stakewise

[saltyfacu]

Description: Eth2 LP on stakewise

Commit (hash):

Tag:

Repo: https://github.com/tapired/StakewiseETH/blob/master/contracts/Strategy.sol

Scope: (List link to files)

• Strategy.sol

Due Diligence: (attach link to due diligence document, see template here)

Additional References or Repo (Link to underlying protocols or relevant documents):

Deployed Contract (etherscan link):

Review Ongoing By:

• @wavey0x
• @fp-crypto

Review Completed By:

• @wavey0x
• @fp-crypto

[saltyfacu]

the approve 0's should not be necessary in constructor. can assume there is already 0 approved.
https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L98

[saltyfacu]

could the max deposit ever be not rounded to a full ether (e.g. 10.5 ether)? if so, this could be an issue.
https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L127

[saltyfacu]

cleanup stray comments

https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L109

https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L143

https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L155

[saltyfacu]

debtPayment may need to be updated when determining profit/loss
https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L178

[saltyfacu]

all above is from @wavey0x

[tapired]

the approve 0's should not be necessary in constructor. can assume there is already 0 approved. https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L98

updated

[tapired]

cleanup stray comments

https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L109

https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L143

https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L155

Updated

[tapired]

could the max deposit ever be not rounded to a full ether (e.g. 10.5 ether)? if so, this could be an issue. https://github.com/tapired/brownie-strategy-mix/blob/5a36f7a575bf3a30dae9125004086f77fd513a78/contracts/Strategy.sol#L127

It can be , rookie mistake from me I updated it.

[wavey0x]

in prepareReturn, you do not account for whether your position has a loss from the adjust position on previous harvest.

In other words, you should compare estimatedTotalAssets() to totalDebt to makes sure a loss has not been realized during previous harvest (in particular on the univ3 swap).

[wavey0x]

you can delete this line and declare uint256 ahead of _amountFreed on line 154

[wavey0x]

use >= on this line to prevent execution from advancing if these values are equal.

[wavey0x]

performing a swap on liquidate position is very dangerous. we should think through this.

StakewiseETH/contracts/Strategy.sol

Line 227 in 1576f8d

IUniV3(uniswapv3).exactInput(

[wavey0x]

critical: we need to enforce a slippage tolerance here.

we already assume that ETH and WSETH2 are equally valued assets. therefore if we are swapping 10 WSETH2, we can allow a minimum amount out of, for example, 9.9 ETH on swap.

slippage tolerance should be configurable values. one for slippage swapping into the asset and another for slippage swapping out of the asset.

StakewiseETH/contracts/Strategy.sol

Line 237 in 1576f8d

0

[wavey0x]

critical: we need to enforce slippage tolerance here as well

StakewiseETH/contracts/Strategy.sol

Line 185 in 1576f8d

0