Badger-Finance / badger-strategies

5 stars 1 forks source link

MAINNET - CVX/bveCVX Curve LP Vault with Briked Strategy #22

Closed Tritium-VLK closed 2 years ago

Tritium-VLK commented 2 years ago

Strategy Review

We have a pool already built on curve. We want to collect liquidity for it, and it currently has no gauge. For now, it would be nice to have a vault with no strategy, that just collected the tokens and holds them to allow badger emissions on the Sett. Once there is liquidity and it can hold peg, it will hopfully start receving curve and convex emissions and will need a strategy setup to farm them.

Description

Allow deposit of crvCVX/bveCVX LP tokens. No strategy for now. 0.1% wd fee.

note that once there is liquidity here @MrrPo has suggested that we move all the convex strats to buying from the pool instead of directly locking.

Code Link

Commit / Hash / Repo should be known from this.

Due Diligence Document (Link, but could also just be a .md file in repo)

This document evaluates the protocols that are interacted with as strategy positions.

Deployed Contract (If present)

Review By

Review by Security Board

Test Checks (screen shot of all test passing)

GalloDaSballo commented 2 years ago

Intial code release: https://github.com/GalloDaSballo/bvecvx-cvx-curve/releases/tag/briked.0

shuklaayush commented 2 years ago

Tests passing. Does nothing. Looks good

image

GalloDaSballo commented 2 years ago

Deploy:

NOTE: I have governance, not prod ready!

Controller Proxy was deployed at: 0x0c41A8613fbeFCC8d6e5dF1020DBb336F875247F

Vault Proxy was deployed at: 0x937B8E917d0F36eDEBBA8E459C5FB16F3b315551

Strategy Logic was deployed at:: 0x27c896a16D27b9d9030a7A1be5C133efB9ea5295 Strategy Proxy was deployed at: 0x98Ca7AFa876f0e15494E76E92C5b3658cdE1Ffe1

GalloDaSballo commented 2 years ago

DEV Controller Wireup:

Transaction sent: 0x4195710a131fbde4b93e529afbbe8497047a509a657bb4549fe19acf8884ce08 Gas price: 104.067092974 gwei Gas limit: 59576 Nonce: 108 Controller.approveStrategy confirmed Block: 13495974 Gas used: 53805 (90.31%)

Transaction sent: 0xf84b2115d5c28ddfd87536d068002377ad72ce1bbaf1c65a836beabe8df7622d Gas price: 101.651716371 gwei Gas limit: 62378 Nonce: 109 Controller.setStrategy confirmed Block: 13495986 Gas used: 56314 (90.28%)

Transaction sent: 0x4a78e67bad849288e6c82e839773e411c50c0f74c170b751807c5c15502f5d1d Gas price: 89.282335042 gwei Gas limit: 59778 Nonce: 110 Waiting for confirmation... /

GalloDaSballo commented 2 years ago

@axejintao @mitche50 See addresses above Contract code is verified

GalloDaSballo commented 2 years ago

Added to registry; https://etherscan.io/tx/0x30a71893ac1db7e1682d66a4e976ccbc72f41b1576258c9b40547a2f3e7f8230

Author is: 0x283C857BA940A61828d9F4c09e3fceE2e7aEF3f7

GalloDaSballo commented 2 years ago

TODO: Set Performance Fees to 0

GalloDaSballo commented 2 years ago

Config:

Changed Fees to 0 setPerformanceFeeStrategist(0) https://etherscan.io/tx/0xfd35ef7dab40c1de3cf680b400fed26154b5878bceed3ba2ed4dc6151685dcb4

setPerformanceFeeGovernance(0) https://etherscan.io/tx/0x0b736b0a1b3fcc03d3fdc51f2e843063919c41229df2a7c37a2bbf62f13cc2d4

sajanrajdev commented 2 years ago

Strategy Review

Bricked Strategy with all operational functions empty (_withdraw, _deposit, _withdrawAll, harvest, and tend).

All tests passing: image

⚠️ Security Consideration ⚠️ The strategy is wired-up to a regular SettV4 vault, which has the earn() function enabled. If this function gets called, the available() assets on it will be transferred to the controller and then to the strategy. Since _deposit() does nothing for the time being and _withdrawSome() is not yet developed on the strat, any want that falls on it will be stuck until these functions get developed - users won't be able to withdraw them.

There are a few options to mitigate this:

  1. Guarantee that nobody calls earn by setting the keeper and governance to the dev_multisig (Keeper set to the Keeper_acl and governance is set to ops_deployer3). - Simplest
  2. Use a bricked version of SettV4 without earn(). - Higher effort
  3. Develop the _withdrawSome() function on the strategy to allow the vault to withdraw from it as needed. - Higher effort

@GalloDaSballo @shuklaayush @dapp-whisperer, please advise.

Other than this it LGTM.

shuklaayush commented 2 years ago

any want that falls on it will be stuck until these functions get developed - users won't be able to withdraw them

I think withdraw from BaseStrategy should still work so funds shouldn't be stuck. Haven't tested it separately though since I assumed it should be covered as part of the test suite https://github.com/GalloDaSballo/bvecvx-cvx-curve/blob/briked.0/deps/BaseStrategy.sol#L190-L219

sajanrajdev commented 2 years ago

I think withdraw still works in the BaseStrategy so we should still be able to withdraw. Haven't tested it separately though since I assumed it should be covered as part of the test suite https://github.com/GalloDaSballo/bvecvx-cvx-curve/blob/briked.0/deps/BaseStrategy.sol#L190-L219

You are right, my bad. I missed the fact that _withdrawSome() is just part of the _withdraw() process. And this line ensures that only the right amount is withdrawn: https://github.com/GalloDaSballo/bvecvx-cvx-curve/blob/07367be5ef617fcb564cc41bf0f385d4350f4614/deps/BaseStrategy.sol#L212

Sorry about that! It LGTM

GalloDaSballo commented 2 years ago

Wireup:

Strategy: BrikedStrategy Vault: Badger Sett Curve.fi Factory Plain Pool: Badger Locked CVX Controller existing or set at: 0x63cF44B2548e4493Fd099222A1eC79F3344D9682 Fees existing or set at: 0, 0, 10 Keeper existing or set at: 0x711A339c002386f9db409cA55b6A35a604aB6cF6 Guardian existing or set at: 0x6615e67b8B6b6375D38A0A3f937cd8c1a1e96386 Strategist existing or set at: 0xB65cef03b9B89f99517643226d76e286ee999e77 Transaction sent: 0xe03b2ab7cab1c99bc09b291352f8c1711347854339f46fe2236d6084fe162a4b Gas price: 0.0 gwei Gas limit: 12000000 Nonce: 117 SettV4.setGovernance confirmed Block: 13501838 Gas used: 31264 (0.26%)

Governance existing or set at: 0xB65cef03b9B89f99517643226d76e286ee999e77

GalloDaSballo commented 2 years ago

Prod Wireup for Controller:

Wrote script for ProdWireup for Controller, requested it be run via: https://github.com/Badger-Finance/badger-ape/issues/499

GalloDaSballo commented 2 years ago

Temporary Prod Wireup:

As Per: https://discord.com/channels/785315893960900629/902283748916203600/903350864482287647

Screenshot 2021-10-28 at 20 45 35

v = SettV4.at("0x937B8E917d0F36eDEBBA8E459C5FB16F3b315551") /Users/entreprenerd/Library/Python/3.9/lib/python/site-packages/brownie/network/contract.py:787: BrownieEnvironmentWarning: 'SettV4' defines a 'balance' function, 'SettV4.balance' is available as SettV4.wei_balance warnings.warn( c = Controller.at("0x0c41A8613fbeFCC8d6e5dF1020DBb336F875247F") v.setController(c, {"from": a[0]}) Transaction sent: 0xc987a233be9fa8ce6c770066b17a1696f170bc068d3253c78affa2c4248fdcf5 Gas price: 260.051724079 gwei Gas limit: 42257 Nonce: 117 SettV4.setController confirmed Block: 13507409 Gas used: 38302 (90.64%)

<Transaction '0xc987a233be9fa8ce6c770066b17a1696f170bc068d3253c78affa2c4248fdcf5'>

c.setGovernance("0xB65cef03b9B89f99517643226d76e286ee999e77", {"from": a[-1]}) Transaction sent: 0x832e80006d53a48608763a0f5d941a832f9874793a07a43f7784b4f317f7327f Gas price: 278.15047648 gwei Gas limit: 37672 Nonce: 118 Controller.setGovernance confirmed Block: 13507418 Gas used: 34199 (90.78%)

<Transaction '0x832e80006d53a48608763a0f5d941a832f9874793a07a43f7784b4f317f7327f'>

v.setGovernance("0xB65cef03b9B89f99517643226d76e286ee999e77", {"from": a[-1]}) Transaction sent: 0x06369f08ad1223b4eec5c7a4b2dd49e2c5796aa3dd8a4a909ac2cc5549c16840 Gas price: 231.222122382 gwei Gas limit: 37745 Nonce: 119 <Transaction '0x06369f08ad1223b4eec5c7a4b2dd49e2c5796aa3dd8a4a909ac2cc5549c16840'>

sajanrajdev commented 2 years ago

strategy.setController(New Controller) executed: https://etherscan.io/tx/0x85aaf422f957be088a4840dbbf61a0ae0e10722e3c09ad0da3e5de2304e0e2e0#eventlog