-
Notifications
You must be signed in to change notification settings - Fork 8
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
[DEPRECATED] Credix Minting/Redeeming/CollectProfit #203
[DEPRECATED] Credix Minting/Redeeming/CollectProfit #203
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.
Hello, looks good! Few things to discuss :)
programs/uxd/src/events.rs
Outdated
pub minting_fee_paid: u64, | ||
} | ||
|
||
/// Event called in [instructions::mint_with_credix_lp_depository::handler]. |
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.
nit:
/// Event called in [instructions::mint_with_credix_lp_depository::handler]. | |
/// Event called in [instructions::redeem_from_credix_lp_depository::handler]. |
programs/uxd/src/events.rs
Outdated
pub redeeming_fee_paid: u64, | ||
} | ||
|
||
/// Event called in [instructions::mint_with_credix_lp_depository::handler]. |
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.
nit:
/// Event called in [instructions::mint_with_credix_lp_depository::handler]. | |
/// Event called in [instructions::collect_profit_of_credix_lp_depository::handler]. |
programs/uxd/src/events.rs
Outdated
/// The collateral amount in native units. (output) | ||
pub collateral_amount_before_fees: u64, | ||
/// The collateral amount in native units. (output) | ||
pub collateral_amount_after_fees: u64, | ||
/// The redeemable issued in native units. (input) | ||
pub redeemable_amount: u64, | ||
/// The fees paid in native units. (output) | ||
pub redeeming_fee_paid: 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.
nit:
collateral_amount_before_fees
and redeeming_fee_paid
shouldn't be labelled as output
?
programs/uxd/src/instructions/credix_lp/register_credix_lp_depository.rs
Show resolved
Hide resolved
// Read useful values | ||
let credix_global_market_state = ctx.accounts.depository.load()?.credix_global_market_state; | ||
let collateral_mint = ctx.accounts.depository.load()?.collateral_mint; | ||
|
||
// Make depository signer | ||
let depository_pda_signer: &[&[&[u8]]] = &[&[ | ||
CREDIX_LP_DEPOSITORY_NAMESPACE, | ||
credix_global_market_state.as_ref(), | ||
collateral_mint.as_ref(), | ||
&[ctx.accounts.depository.load()?.bump], | ||
]]; |
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.
nit, theses values are used solely for generating depository pda signer
// Read useful values | |
let credix_global_market_state = ctx.accounts.depository.load()?.credix_global_market_state; | |
let collateral_mint = ctx.accounts.depository.load()?.collateral_mint; | |
// Make depository signer | |
let depository_pda_signer: &[&[&[u8]]] = &[&[ | |
CREDIX_LP_DEPOSITORY_NAMESPACE, | |
credix_global_market_state.as_ref(), | |
collateral_mint.as_ref(), | |
&[ctx.accounts.depository.load()?.bump], | |
]]; | |
// Make depository signer | |
let credix_global_market_state = ctx.accounts.depository.load()?.credix_global_market_state; | |
let collateral_mint = ctx.accounts.depository.load()?.collateral_mint; | |
let depository_pda_signer: &[&[&[u8]]] = &[&[ | |
CREDIX_LP_DEPOSITORY_NAMESPACE, | |
credix_global_market_state.as_ref(), | |
collateral_mint.as_ref(), | |
&[ctx.accounts.depository.load()?.bump], | |
]]; |
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.
nice catch!
programs/uxd/src/state/controller.rs
Outdated
current_size < MAX_REGISTERED_CREDIX_LP_DEPOSITORIES, | ||
UxdError::MaxNumberOfCredixLpDepositoriesRegisteredReached | ||
); | ||
// Increment registered Mango Depositories count |
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.
nit:
// Increment registered Mango Depositories count | |
// Increment registered Credix LP Depositories count |
programs/uxd/src/state/controller.rs
Outdated
.registered_credix_lp_depositories_count | ||
.checked_add(1) | ||
.ok_or_else(|| error!(UxdError::MathError))?; | ||
// Add the new Mango Depository ID to the array of registered Depositories |
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.
nit:
// Add the new Mango Depository ID to the array of registered Depositories | |
// Add the new Credix LP Depository ID to the array of registered Depositories |
tests/utils.ts
Outdated
export async function createCredixLpDepositoryDevnetUSDC() { | ||
return await CredixLpDepository.initialize({ | ||
connection: getConnection(), | ||
uxdProgramId: uxdProgramId, | ||
collateralMint: new PublicKey("Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr"), | ||
collateralSymbol: "USDC(CredixDevnet)", | ||
credixProgramId: new PublicKey("CRdXwuY984Au227VnMJ2qvT7gPd83HwARYXcbHfseFKC"), | ||
credixMultisig: new PublicKey("4u825MpPxsRSxnvAJ8jJRsvAtbXByLhZZTWjEg1Kcjkd"), | ||
credixTreasuryCollateral: new PublicKey("BDWiqHSTpZAWcERiQuZFQJxich22J5uy9ALXLQ3eryWG"), | ||
credixMarketName: "credix-marketplace", | ||
profitTreasury: new PublicKey("aca3VWxwBeu8FTZowJ9hfSKGzntjX68EXh1N9xpE1PC"), | ||
}); | ||
} |
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.
nit:
export async function createCredixLpDepositoryDevnetUSDC() { | |
return await CredixLpDepository.initialize({ | |
connection: getConnection(), | |
uxdProgramId: uxdProgramId, | |
collateralMint: new PublicKey("Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr"), | |
collateralSymbol: "USDC(CredixDevnet)", | |
credixProgramId: new PublicKey("CRdXwuY984Au227VnMJ2qvT7gPd83HwARYXcbHfseFKC"), | |
credixMultisig: new PublicKey("4u825MpPxsRSxnvAJ8jJRsvAtbXByLhZZTWjEg1Kcjkd"), | |
credixTreasuryCollateral: new PublicKey("BDWiqHSTpZAWcERiQuZFQJxich22J5uy9ALXLQ3eryWG"), | |
credixMarketName: "credix-marketplace", | |
profitTreasury: new PublicKey("aca3VWxwBeu8FTZowJ9hfSKGzntjX68EXh1N9xpE1PC"), | |
}); | |
} | |
export async function createCredixLpDepositoryDevnetUSDC(): Promise<CredixLpDepository> { | |
return CredixLpDepository.initialize({ | |
connection: getConnection(), | |
uxdProgramId, | |
collateralMint: new PublicKey("Gh9ZwEmdLJ8DscKNTkTqPbNwLNNBjuSzaG9Vp2KGtKJr"), | |
collateralSymbol: "USDC(CredixDevnet)", | |
credixProgramId: new PublicKey("CRdXwuY984Au227VnMJ2qvT7gPd83HwARYXcbHfseFKC"), | |
credixMultisig: new PublicKey("4u825MpPxsRSxnvAJ8jJRsvAtbXByLhZZTWjEg1Kcjkd"), | |
credixTreasuryCollateral: new PublicKey("BDWiqHSTpZAWcERiQuZFQJxich22J5uy9ALXLQ3eryWG"), | |
credixMarketName: "credix-marketplace", | |
profitTreasury: new PublicKey("aca3VWxwBeu8FTZowJ9hfSKGzntjX68EXh1N9xpE1PC"), | |
}); | |
} |
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.
sg 👌
); | ||
|
||
// Assumes and enforce a collateral/redeemable 1:1 relationship on purpose | ||
let collateral_amount_before_precision_loss: u64 = u64::try_from(profits_redeemable_amount) |
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.
Is profits_redeemable_amount
the profits that may effectively be withdrawn?
Imagine a user deposits 10 USDC worthing 4 LP Token.
5 days later, 4 LP Token worth 11 USDC.
You call collect profit, profits_redeemable_amount
should be 0 because the LP tokens are still locked, correct?
Imagine another scenario:
If someone redeem from the pool, it could potentially redeem for any unlocked LP token, so potentially from rewards. Leeding to rewards not being collectible.
imagine 5000 UXD being minted for 50 Lp tokens, with 10 Lp tokens unlocked (90d+ in the pool). An user could redeem for 1200 UXD and uses all unlock Lp tokens.
If we want to collect profits, we couldn't, because no Lp token is available to swap.
To fix that, in redeem function, we could calculate the profit amount and prevent users to redeem for it.
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.
Good question!! might thought on this are:
- this will become irrelevant after 30 days as there are no staggered locking
- I think we could abstain from collecting profit until we think enough if unlocked
- I dont know if we need to provide any extra guarantee past the: if you wait 30 days you can redeem your money anyway
We can chat by voice about this
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 will become irrelevant after 30 days as there are no staggered locking
You mean if I put 10 USDC in credix, after 30 days, if I put an other 20 USDC, 1 day later I could withdraw the 30 USDC? New deposits doesn't induce new lock?
I think we could abstain from collecting profit until we think enough if unlocked
Ideally I think we should tend to an "automatic" profit collection every X days from every depositories, using something like snowflakes. Would prefer to not have to think about when to unlock.
|
||
[registry] | ||
url = "https://anchor.projectserum.com" | ||
|
||
[scripts] | ||
# The quick version for development - Keep this version as the CI swap this line for its needs | ||
test = "npx ts-mocha -p ./tsconfig.json -t 500000 tests/test_development.ts --reporter mochawesome --require mochawesome/register --reporter-options quiet=true,reportTitle=uxdprogram-test_integration --trace-warnings" | ||
test = "npx ts-mocha -p ./tsconfig.json -t 500000 tests/test_ci_credix_lp.ts --reporter mochawesome --require mochawesome/register --reporter-options quiet=true,reportTitle=uxdprogram-test_integration --trace-warnings" |
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.
pls revert this to test_development
, ci search for this file name and replace with different ci test files for the anchor test job
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.
Agreed king, I also need to revert the local keys, this branch is an all-inclusive branch, ill make a polished-ready-to-audit PR with minimal changes
deprecated by: |
PR Combo: