-
Notifications
You must be signed in to change notification settings - Fork 3
[WIP] Leveraged Basis trade vault #64
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
Conversation
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.
LGMT, just some questions + nitpicks.
Feel free to not implement nitpicks into this basis trade deploy - but mostly leaving comments so we don't forget for next deploy.
bytes32 hash = getActionTypedDataHash(action); | ||
// TODO: check we want caching. **If params are updated, old ones would not be revoked.** | ||
// if (_getBaseSigningTSAStorage().signedData[hash]) return; |
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.
can remove now?
tradeHelperVars.basePrice = _getBasePrice(); | ||
tradeHelperVars.perpPrice = _getPerpPrice(); | ||
(tradeHelperVars.perpPosition, tradeHelperVars.baseBalance, tradeHelperVars.cashBalance) = _getSubAccountStats(); | ||
tradeHelperVars.underlyingBase = _getConvertedMtM(); |
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 noticed this function actually reverts if mTm < 0. I think it was initially done to prevent deposits / withdrawals, but now that it's used in this generic helper, should we move this outside.
E.g. would this block any valid action / deleveraging when vault is in a dangerous situation?
if (convertedMtM < 0 || margin < 0) {
revert CMTSA_PositionInsolvent();
}
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.
If MtM < 0, its actually in the vault's interest to get liquidated before depositing, since you'd be paying off insolvency using new funds.
Will remove the margin check though, since if it is flagged, the action will be blocked anyways.
Does this work for when the last user tries to withdraw? function _verifyTradeDelta(TradeHelperVars memory tradeHelperVars, int amtDelta) internal view {
LBTSAStorage storage $ = _getLBTSAStorage();
uint perpBaseRatio = tradeHelperVars.perpPrice.divideDecimal(tradeHelperVars.basePrice);
int deltaChange = tradeHelperVars.isBaseTrade ? amtDelta : amtDelta.multiplyDecimal(int(perpBaseRatio));
// delta as a % of TVL
int portfolioDeltaPercent = (
int(tradeHelperVars.baseBalance) + tradeHelperVars.perpPosition.multiplyDecimal(int(perpBaseRatio))
).divideDecimal(int(tradeHelperVars.underlyingBase));
int newDeltaPercent = portfolioDeltaPercent + deltaChange.divideDecimal(int(tradeHelperVars.underlyingBase));
int targetDelta = $.lbParams.deltaTarget;
if ((newDeltaPercent - targetDelta).abs() < (portfolioDeltaPercent - targetDelta).abs()) {
// delta is improving
return;
}
require(
newDeltaPercent >= targetDelta - $.lbParams.deltaTargetTolerance
&& newDeltaPercent <= targetDelta + $.lbParams.deltaTargetTolerance,
LBT_PostTradeDeltaOutOfRange()
);
} e.g. if It makes me think that the actual calculation might not be optimal? Here are some cases:
I think the TLDR is - the smaller the TVL the less you are allowed to trade / withdraw at a time. Not sure if that's intentional + not sure if that breaks for last withdrawal |
oof good catch - i think the issue here is that the denominator once that's resolved then for 1.0 target delta vault all withdrawals always pass the delta check (but are still subject to leverage check), and with non-1.0 delta you still need to split them up and close the hedges in-between, but that's expected |
0737eba
to
fc1535b
Compare
No description provided.