-
Notifications
You must be signed in to change notification settings - Fork 71
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
added liquidate and redeem (with protocol fee) #75
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.
My impression of this change is that you want to achieve two things:
- Add a convenience method for liquidate + redeem
- Add liquidation fees to generate protocol revenue
It looks to me that the convenience method is not much (if at all) more compute efficient than calling the individual instructions separately. However, it seems like you can cut out a refresh reserve ix and you're probably saving on encoded tx size as well, so maybe it's worth it? I would have expected more effort spent on optimizing this instruction to avoid redundant work but maybe you plan to do that later?
For the protocol fees, since there are no fees for the original liquidate instruction, I'm curious what your plans are for enticing liquidators to use the new ix over the old one. Any reason you didn't add a collateral token fee receiver to the reserve to collect fees there too?
let withdraw_liquidity_amount = _redeem_reserve_collateral( | ||
program_id, | ||
withdraw_collateral_amount, | ||
destination_collateral_info, |
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.
Rather than withdrawing collateral during _liquidate_obligation
and then burning the collateral in _redeem_reserve_collateral
, you could save a CPI by just directly burning from the reserve's collateral supply. And on this subject, there is some other redundant work done in both _liquidate_obligation
and _redeem_reserve_collateral
(notably the PDA creation) that could be saved with some more work as well.
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.
yeh I think the reasoning i had for doing this way was to be more modular and write as little new math/logic code as possible not optimize compute (beyond sticking to < 200k ish). but perhaps there is some level of optimization that could be added.
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.
Makes sense, optimizations can easily introduce critical bugs as well, so there's a trade-off
Yeh it's probably less compute efficient (but maybe not since saves a refresh) and saves a few bytes of txn space. also it's slightly challenging for liquidators to know exactly how much ctoken they will receive so this allows us to compute that for them so they don't have to guess and be left with some dust (or do a follow up txn which is what we observe now mostly) edit:
the plan is to disable the old method after X amount of time |
Cool, that's what I expected. I would just be wary of the increased compute cost then, but like I've mentioned a few times, can always do that later as long as there's no big impact to how liquidators are currently operating. |
No description provided.