[Breaking] feat: self-redeem #702
Conversation
| pub(crate) fn execute<T: Config>(vault_id: DefaultVaultId<T>, amount_wrapped: Amount<T>) -> DispatchResult { | ||
| // ensure that vault is not liquidated and not banned | ||
| ext::vault_registry::ensure_not_banned::<T>(&vault_id)?; | ||
|
|
||
| // for self-redeem, dustAmount is effectively 1 satoshi | ||
| ensure!(!amount_wrapped.is_zero(), Error::<T>::AmountBelowDustAmount); | ||
|
|
||
| let (fees, consumed_issued_tokens) = calculate_token_amounts::<T>(&vault_id, &amount_wrapped)?; | ||
|
|
||
| take_user_tokens::<T>(&vault_id.account_id, &consumed_issued_tokens, &fees)?; | ||
|
|
||
| update_vault_tokens::<T>(&vault_id, &consumed_issued_tokens)?; | ||
|
|
||
| Pallet::<T>::deposit_event(Event::<T>::SelfRedeem { | ||
| vault_id, | ||
| amount: consumed_issued_tokens.amount(), | ||
| fee: fees.amount(), | ||
| }); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
My original code looked surprisingly complex for such a simple function, so this was a bit of an experiment to attempt to make the code easier to read. I like how clear this function is now, although I did end up paying the price in increased verbosity.
sander2
force-pushed
the
feat/self-redeem
branch
from
d47620b
to
65db349
Compare
| // for self-redeem, dustAmount is effectively 1 satoshi | ||
| ensure!(!amount_wrapped.is_zero(), Error::<T>::AmountBelowDustAmount); | ||
|
|
||
| let (fees, consumed_issued_tokens) = calculate_token_amounts::<T>(&vault_id, &amount_wrapped)?; |
There was a problem hiding this comment.
A potentially nice refactoring from a concept I came across whilst digging into Bitcoin Core is to bundle this tuple into something similar to the COutput and then calculate the amount to burn using GetEffectiveValue.
There was a problem hiding this comment.
This function is only used within this module (and only once at that), so I don't think that refactoring is worth it
There was a problem hiding this comment.
Yeah just mentioning it because I thought it was pretty ;)
|
|
||
| #[pallet::weight(<T as Config>::WeightInfo::self_redeem())] | ||
| #[transactional] | ||
| pub fn self_redeem( |
There was a problem hiding this comment.
Should we bundle this into the existing redeem flow? I guess having a separate extrinsic is cleaner but not sure if there were other considerations in your design here
There was a problem hiding this comment.
I don't think that would be very clean. The existing request_redeem has a btc address and a vault account id, both of which are not used for self_redeem. And it's very different in operation in that no request is placed into storage at all (UI will have to handle this differently).
What do you mean? There are integration tests, see |
Yes you are right, sorry I must have missed those - trying to do too many things at once |
(User story copied from notion)
Note that the user still pays the 0.5% redeem fees unless it redeems the full amount left in the vault