-
Notifications
You must be signed in to change notification settings - Fork 60
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
WIP: unlendable deposits #907
base: dev
Are you sure you want to change the base?
Conversation
- for sanity, the liquidator likely must have lendable positions too - instruction for creating and closing an unlendable position - heaps of tests, rs client changes, ts client changes :/ Done: - when dealing with the vault, always keep unlendable_deposits in the vault (unless withdrawing a no-lending position) - no-lending positions can't work as settle token positions for perps
need to audit all uses still
@@ -331,6 +332,14 @@ pub fn serum3_place_order( | |||
)? | |||
}; | |||
|
|||
// Verify that the no-lending amount on the vault remains. If the acting account | |||
// itself had a no-lending position, the unlendable_deposits has already been reduced. |
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.
this is a new constraint that needs to be factored into max-obv1-size
// itself had a no-lending position, the unlendable_deposits has already been reduced. | ||
require_gte!( | ||
change.vault_balance, | ||
bank.unlendable_deposits, |
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.
this is a constraint that needs to appear in max-swap?
ctx.accounts | ||
.vault | ||
.amount | ||
.saturating_sub(bank.unlendable_deposits), |
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.
ah this is scary, how can we avoid using vault amounts just as is in future
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've pushed more changes to the ts side where it was using vaults
Should flash loaners be allowed to borrow unlendable deposits? |
technically yes, but we should just be conservative imo, or? |
if position.allow_lending() { | ||
assert!(position.unlendable_deposits == 0); | ||
let opening_indexed_position = position.indexed_position; | ||
let result = self.deposit_internal(position, native_amount, allow_dusting, now_ts)?; |
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 would rename the deposit_internal
method to make it explicit that it's only for "lendable position"
if position.allow_lending() { | ||
assert!(position.unlendable_deposits == 0); | ||
let opening_indexed_position = position.indexed_position; | ||
let res = self.withdraw_internal( |
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.
Same here, would rename withdraw_internal
to a more explicit withdraw_indexed_pos_internal
or withdraw_lendable_pos_internal
MangoError::UnlendableTokenPositionCannotBeNegative | ||
); | ||
position.unlendable_deposits -= withdraw_amount_u64; | ||
self.unlendable_deposits -= withdraw_amount_u64; |
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.
Should/could we also add a require_gte on the bank unlendable deposit vs withdrawn amount ?
TODO: