search

harvest-finance / harvest

Bread for the people!
217 stars 94 forks source link

Update weth strat withdraw logic #4

Closed Byron-McKeeby closed 4 years ago

breadforthepeople commented 4 years ago

Thank you for the fix. We have created a test for the strategy on mainnet fork, and it works well. The same check is needed in redeemMaximum() as well, for the corner case when the execution path goes through the withdrawAllToVault(...). Can you please cover this case too?

Byron-McKeeby commented 4 years ago

For redeemMaximum should we instead query the market for available cash and then redeem exactly that amount? That behavior would not redeem all crWETH (there is no situation in which we can guarantee total redemption). But it would redeem the maximum amount of crWETH, which feels like the best behavior

breadforthepeople commented 4 years ago

This seems like a reasonable solution. Can you please add this?

Byron-McKeeby commented 4 years ago

This seems like a reasonable solution. Can you please add this?

Added