-
Notifications
You must be signed in to change notification settings - Fork 3
generalisedTSA and BaseTSA performance fee #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
joshpwrk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are some edge cases because of how the non-atomic perfFees work w.r.t requestWithdrawal()
| /////////////////////// | ||
| // Action Validation // | ||
| /////////////////////// | ||
| function _verifyAction(IMatching.Action memory action, bytes32 actionHash, bytes memory extraData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an idea for the future (def not this PR) - where you can build your own vault by adding on whatever guardrails you want.
For example, each ActionType has an "array" of hooks that get called in order.
Every vault would effectively just be a the Generalized Vault + a subsection of guardrails:
- depositGuardrails[ ]
- withdrawalGuardrails[ ]
- tradeGuardrails[ ]
- collateralGuardrails[ ]
- rfqGuardrails[ ]
Something like that might be much easier to build a "self-serve UX" for.
So the CoveredCallTSA would now just be:
GeneralizedTSA +
- depositGuardrails: [weekly deposit guardrail]
- withdrawGuadrails: [weekly withdraw guardrail]
- tradeGuardrails: [premium price guardrail, expiry guardrail]
- collateralGuardrails: [minimum collateral to cover short guardrail]
- rfqGuardrails: None
|
|
||
| if (block.timestamp >= lastSnapshotTime + $.tsaParams.performanceFeeWindow) { | ||
| address feeRecipient = $.tsaParams.feeRecipient; | ||
| (uint perfFee, uint sharePrice) = _getPerfFee(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: would remove uint sharePrice since it's not doing anything
| } | ||
| } | ||
|
|
||
| function _collectWithdrawalPerfFee(uint sharesBurnt, uint withdrawAmount, uint perfFee, uint currentSharePrice) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the _burn() and collection of perf fees happens non-atomically, I think there are two edge cases:
(1) the share price is calculated by doing totalSupply() / _accountValue(), but because requestWithdrawal() burns all withdraw shares, later the share price DURING the processWithdrawal() call will be higher than true value (esp. for big withdrawals).
This means big withdrawers would pay way higher perf fees and depositors that deposit during this dead space basically have high slippage.
(2) Can users avoid paying perf fees by timing their withdrawals? Let's say the vault has a weekly perf window and it just made 50%. As a user, I think I would:
requestWithdrawlike 1-30sec before the end of the window.- Hopefully, the keeper doesn't process my withdrawal before window ends.
- window ends - I call
collectFees(), which resets the$.lastShareValue - By the time the share keeper gets to my withdrawal, the perfFee is 0% since storage values were reset.
|
One random thought i had for the perf fee: might be nice to have a "baseline" performance number and have the fee be a % of For example, the vault might expect a baseline expected growth of 5% APY, and perf fee should only be charged on w/e profits above 5% This seems pretty common in tradfi: https://corporatefinanceinstitute.com/resources/career-map/sell-side/capital-markets/2-and-20-hedge-fund-fees/
|
No description provided.