FIP-33b: Move BAL to 80% BAL / 20% WETH pool (new Balancer WeightedPool deposit) #373
Conversation
proposals/description/fip_999.ts
Outdated
| target: 'collateralizationOracle', | ||
| values: '0', | ||
| method: 'swapDeposit(address,address)', | ||
| arguments: ['{balDepositWrapper}', '{balancerDepositBalWeth}'], |
There was a problem hiding this comment.
Should we add a WETH lens?
There was a problem hiding this comment.
I guess fully reporting in BAL is cool
There was a problem hiding this comment.
Spicier: what will we do for the 80% TRIBE 20% WETH pool ?
proposals/dao/fip_999.ts
Outdated
| '0x5c6ee304399dbdb9c8ef030ab642b10820db8f56000200000000000000000014', // poolId | ||
| '300', // max 3% slippage | ||
| '0xba100000625a3754423978a60c9317c58a424e3D', // BAL token | ||
| [addresses.balUsdCompositeOracle, addresses.chainlinkEthUsdOracleWrapper] |
There was a problem hiding this comment.
have you verified the oracle ordering both in the array and price reporting?
There was a problem hiding this comment.
yes, will make it more clear with additional tests / invariant checks in validation step
| } | ||
|
|
||
| /// @notice redeeem all assets from LP pool | ||
| function exitPool() external whenNotPaused onlyPCVController { |
There was a problem hiding this comment.
why not include a to address?
There was a problem hiding this comment.
because we want to burn the FEI if the pool contains FEI, and if we exitPool on a target directly, it may not be able to burn FEI held or report it properly as protocol-owned. That was my reasoning
There was a problem hiding this comment.
Still think we should give ourselves the option
| } | ||
|
|
||
| /// @notice claim BAL rewards associated to this PCV Deposit. | ||
| /// Note that if dual incentives are active, this will only claim BAL rewards. |
There was a problem hiding this comment.
Is it possible to handle dual incentives case?
There was a problem hiding this comment.
we can pass an array of distributionIds, amounts, merkle proofs, token addresses, and token distributors, but at this point we might as well call the MerkleOrchard directly 😅 I'm not very convinced about this claimRewards function btw, there are so many parameters, it's going to be more convenient to make an offline documentation on how to claim directly on the merkle orchard (or make a front-end / use Balancer's front-end after they allow to claim for another address)
| } | ||
|
|
||
| // @notice returns the manipulation-resistant balance of tokens & FEI held. | ||
| function resistantBalanceAndFei() public view override returns ( |
There was a problem hiding this comment.
How do you feel about just reusing Lenses for this instead of duplicating code?
There was a problem hiding this comment.
why not. We'd have to improve the Lens to handle more than 2 assets and be able to account only the liquidity owned by the PCVDeposit, and not the whole pool content. But it's doable
|
I'd also like to see an integration test probably more than one |
Also, only move the 200,000 BAL, and don't create new pools etc
eswak
changed the title
|
Added a bunch of e2e tests to use the PCVDeposit in various situations, and took Jeff's review into account
|
FIP-33b
Move the BAL from the Timelock to the 80% BAL / 20% WETH pool.
New contracts :
BalancerPCVDepositBase
This PCV Deposit is abstract, and intended to be inherited by deposits for Balancer WeightedPools, StablePools, MetaStablePools, etc.
It can :
BalancerPCVDepositWeightedPool
This PCV Deposit is intended to deposit PCV in Balancer WeightedPools (2 tokens, i.e. PoolTwo, or more).
An oracle price feed for each tokens of the pool is required.
Accounting is made in one of the tokens of the pool.
FEI can be minted to match the other tokens deposited - if the deposit is granted minter role.
Unit tests :
