Round trip fee guardrail in Recovery Mode #1098
Conversation
EndymionJkb
changed the title
There was a problem hiding this comment.
LGTM!
This is super overkill, but I'm not against it. I'd love @danielmkm's thoughts on this one.
| @@ -744,6 +744,14 @@ contract VaultExtension is IVaultExtension, VaultCommon, Proxy { | |||
| return _isPoolInRecoveryMode(pool); | |||
| } | |||
|
|
|||
| // Needed to avoid stack-too-deep.apply | |||
There was a problem hiding this comment.
I tried using a struct for the removeLiquidityRecovery function itself first, as we have elsewhere, but it wasn't enough.
| vault.manualEnableRecoveryMode(pool); | ||
| // Set the fee, and flag to trigger collection of it. | ||
| vault.manualSetStaticSwapFeePercentage(pool, BASE_MAX_SWAP_FEE); | ||
| vault.manualSetAddLiquidityCalledFlag(pool, true); |
There was a problem hiding this comment.
You don't need this one actually. The whole test runs in the same tx.
Maybe just leave a comment, and / or assert that the flag is true?
There was a problem hiding this comment.
I did this, but it doesn't seem to be testing the content of the events properly. If that flag is set, it should be charging fees in the original test too, but the expected event doesn't have them, yet it passes. I manually clear the flag in the one that's not supposed to have the roundtrip fee, and that works too.
Testing the balances verifies that the two cases behave differently, but I wonder where else events aren't getting tested.
Description
Apply the same roundtrip fee guardrail on recovery mode that we do with regular proportional withdrawals. If there was a preceding add liquidity operation (which there wouldn't normally be in a recovery mode withdrawal), charge the pool's static swap fee.
To keep recovery mode as simple as possible, this fee is charged to the user (i.e., they receive fewer tokens for their BPT), but the tokens simply remain in the pool, accruing value to the other LPs.
Type of change
Checklist:
main, or there's a description of how to mergeIssue Resolution
Resolves #1094