Scaffold LSD Depository #280
Conversation
programs/uxd/src/instructions/lsd/borrow_from_lsd_depository.rs
Outdated
Show resolved
Hide resolved
| bump, | ||
| payer = payer, | ||
| )] | ||
| pub profits_token_account: Account<'info, TokenAccount>, |
There was a problem hiding this comment.
if we design to have a borrow fee settings here,
then it also require a token account storing the redeemable that the user pay upon borrowing?
And same with repay fee.
So in total there would be redeemable_profits_token_account, collateral_profits_token_account, liquidation_profits_token_account ,
and each of their own profits beneficiary
There was a problem hiding this comment.
Just thinking based on my reading of the rest of the PR:
- this ATA is probably going to be in "liquidation_mint" amount so more like "profits_liquidation"
- the user most likely will pay fees using collateral rather than redeemable right ? or we just mint "less" redeemable ? I dont know if we need to store any redeemable inside of the depository
There was a problem hiding this comment.
Depends on how we wanna store fee as, but like for solend if there are a borrow fee, then its collected on the borrowing mint which is redeemable on our case, like if u are borrowing 10k UXD and borrowing fee on 0.05% then 10k is minted but u only take home 9995, and we take the 5 remaining. To repay the whole position u have to mint/swap elsewhere 5 more.
But agree "mint less" is a easier way.
There was a problem hiding this comment.
Yhea, for now I factored it all into a profits token account and I left a comment somewhere "swap to profit mint" on collection
…olanaLabs and other players
| pub collateral_amount: u64, | ||
| pub redeemable_amount: u64, | ||
| pub effective_ltv_bps: u8, | ||
| pub liquidation_price: u64, |
There was a problem hiding this comment.
nit: is the liquidation price in liquidation_mint amount ? if so maybe it could be in the name of the variable ?
There was a problem hiding this comment.
default price is in the target mint, if specified _usd it's in USD I would say?
| pub collateral_mint_decimals: u8, | ||
| pub liquidation_mint: Pubkey, // when liquidation occurs, this is the target mint (should ) | ||
| pub liquidation_mint_decimals: u8, | ||
| pub profits_token_account: Pubkey, // using liquidation_mint. Contains liquidation profits + borrowing fees if any |
There was a problem hiding this comment.
using the naming used in other places this could be: depository_liquidation ?
| pub depository: Pubkey, | ||
| pub collateral_amount: u64, | ||
| pub redeemable_amount: u64, | ||
| pub effective_ltv_bps: u8, |
There was a problem hiding this comment.
qq: im not sure I understand the purpose of this field
There was a problem hiding this comment.
Yes me neither, removed a few minutes ago before seeing that comment 😆
| /// #4 | ||
| #[account( | ||
| mut, | ||
| seeds = [LSD_DEPOSITORY_NAMESPACE, collateral_mint.key().as_ref()], |
There was a problem hiding this comment.
should we include the liquidation mint in the depository seeds?
There was a problem hiding this comment.
That would mean running two LSD depository for the same collateral with different liquidation mints... which we won't do 99.999% chance. Tbh if USDC dies, we die 😓
There was a problem hiding this comment.
yeah fair enough, I guess my point was: is there any downside to include it? no strong opinion tho
| payer = payer, | ||
| bump, | ||
| )] | ||
| pub lsd_position_collateral_token_account: Box<Account<'info, TokenAccount>>, |
There was a problem hiding this comment.
micro-nit: in other places in the code, we would just call this:
lsd_position_collateral
| "editor.formatOnSave": true, | ||
| "rust-analyzer.linkedProjects": [ | ||
| "./programs/uxd/Cargo.toml" | ||
| ] |
There was a problem hiding this comment.
interdasting, im curious what this is for
There was a problem hiding this comment.
Me too, vscode what have you done 😄
|
quick nit: should we target this to |
| { | ||
| // - 0.1 verify that borrows are enabled | ||
| let lsd_depository = ctx.accounts.depository.load()?; | ||
| require!(lsd_depository.borrowing_disabled == false, UxdError::BorrowingDisabled); |
There was a problem hiding this comment.
nit: probably can be done in a validate function (we usually use the validate pattern for this stuff)
There was a problem hiding this comment.
Hehe aware of that I wrote it 😏
Normally the validate function, as the name suggest, validates the input parameters. For convenience we let the program frozen get there, which is not really related.
For business checks I find it clearer to keep them in the body of the function
| } | ||
|
|
||
| // - 2 [MINT REDEEMABLES] -------------------------------------------------- | ||
| let redeemable_borrow_amount = collateral_amount / u64::from(loan_to_value_bps) * u64::from(BPS_POWER); |
There was a problem hiding this comment.
might want to use checked_math there, i'd feel safer
There was a problem hiding this comment.
yes need to update to checked everywhere
There was a problem hiding this comment.
Updating these in the other PR, thanks for the feedback
| curtime, | ||
| false, | ||
| )?; | ||
| let lsd_price_usd = lsd_price.get_asset_amount_usd(collateral_amount, depository.collateral_mint_decimals)?; |
There was a problem hiding this comment.
wow that is a thing? how do they compute this?? Very cool!
| false, | ||
| )?; | ||
| let lsd_price_usd = lsd_price.get_asset_amount_usd(collateral_amount, depository.collateral_mint_decimals)?; | ||
| liquidation_price = maths::checked_mul(redeemable_borrow_amount, maths::checked_div(u64::from(depository.max_loan_to_value_bps), u64::from(BPS_POWER))?)?; |
There was a problem hiding this comment.
sounds like we need a utility function to compute this, its used in many places
There was a problem hiding this comment.
Just 2 op I rather keep things flat x)
I know we diverge on this, but I think it's more straight forward. There might be a function in the math lib already though, else I would stick to what's there
No description provided.