Introduce RecoveryModeHelper #2068
Conversation
There was a problem hiding this comment.
Using cash is good future-proofing for when we start using asset managers for various unseeded things (managed add/remove, LBPs, etc.) Comments inline.
This should be highlighted in the documentation though, since recovery UIs or users on Etherscan would likely not think of this, and expect values proportional to the total balances
|
|
||
| bptAmountIn = userData.recoveryModeExit(); | ||
|
|
||
| amountsOut = WeightedMath._calcTokensOutGivenExactBptIn(cashBalances, bptAmountIn, virtualSupply); |
There was a problem hiding this comment.
In the same vein as the above comment about overfitting to pool types, we have several versions of the _calcTokensOutGivenExactBptIn function (and with stable it's embedded in the recovery mode exit). Since it's external anyway, maybe implement a version locally here (like the old _computeProportionalAmountsOut) for all pool types to use, so that we avoid using libraries specific to weighted pools?
There was a problem hiding this comment.
Yes, I want to have those in the library for recovery mode user data decoding. I didn't do that yet because it'll cause conflicts with the stable composable branch merge, but we should do it once that's in.
Co-authored-by: EndymionJkb <[email protected]>
| interface IRecoveryModeHelper { | ||
| /** | ||
| * @dev Computes a Recovery Mode Exit BPT and token amounts for a Pool. Only 'cash' balances are considered, to | ||
| * avoid scenarios where the last LPs to attempt to exit the Pool cannot because only 'managed' balance remains. |
There was a problem hiding this comment.
| * avoid scenarios where the last LPs to attempt to exit the Pool cannot because only 'managed' balance remains. | |
| * avoid scenarios where the last LPs to attempt to exit the Pool cannot do it because only 'managed' balance | |
| * remains. |
| _vault = vault; | ||
| } | ||
|
|
||
| function getVault() public view returns (IVault) { |
There was a problem hiding this comment.
Maybe consider adding this to the interface?
Perhaps it's not necessary, but there are other interfaces where we do include it.
|
|
||
| bptAmountIn = userData.recoveryModeExit(); | ||
|
|
||
| amountsOut = WeightedMath._calcTokensOutGivenExactBptIn(cashBalances, bptAmountIn, virtualSupply); |
| async updateManaged(poolId: string, managedl: BigNumberish[]): Promise<ContractTransaction> { | ||
| return this.instance.updateManaged(poolId, managedl); |
There was a problem hiding this comment.
| async updateManaged(poolId: string, managedl: BigNumberish[]): Promise<ContractTransaction> { | |
| return this.instance.updateManaged(poolId, managedl); | |
| async updateManaged(poolId: string, managed: BigNumberish[]): Promise<ContractTransaction> { | |
| return this.instance.updateManaged(poolId, managed); |
There was a problem hiding this comment.
Good catch! I didn't see that one
Description
This introduces an external helper contract that can be used to compute recovery mode exits. Unlike all exits we've had so far, it uses the 'cash' balance of the Pool exclusively. By placing this code outside of the Pool, we can reduce its bytecode size.
This initial version only works with Composable Pools, but it's easily extendable to also work with regular Pools. On a later PR I'll modify ManagedPool so that it uses this helper.
Type of change
Checklist:
master, or there's a description of how to mergeIssue Resolution
Part of #2042