Mandatory replace requests

[nud3l]

Is your feature request related to a problem? Please describe.
Any vault that provides unused mint capacity is subject to the ‘risk’ of receiving an issue request that can fill up all its remaining mint capacity. Hence, a replace request by another vault is nothing else than an issue request by a user (and hence should charge the same fees) but helps vaults at risk to mitigate liquidations.

Describe the solution you'd like
If a vault can request a replace by another vault with certainty, there is much lower risk of getting liquidated. If the vault which is supposed to replace the original vault has an option to decline this request, that option of risk mitigation becomes quite meaningless.

Additional context
We can achieve this functionality without changing too much in the replace core model.

• As is: oldVault announces to want to replace X BTC
• Changed: oldVault can accept replace request on behalf of newVault (similar to how on issue the vault is not involved but rather the user makes the request and the vault cannot decline)
• As is: execution of the request.

[sander2]

If we want mandatory replace requests, we can simplify replaces a lot by removing the request_replace and withdraw_replace extrinsics, and modifying accept_replace such that it can be called by the old vault (and probably rename the function at that point)

[nud3l]

Yes, I think that is ideal.

[nakul1010]

Following is the proposal for the mandatory replace request

1. accept_replace extrinsic
   • Parameters
     • old_vault:AccountID
     • new_vault:VaultID
     • amount: BalanceOf<T> // amount in IBTC
     • currency_pair: DefaultVaultCurrencyPair<T>

Flow Diagram

[sander2]

I think the following steps need to be done. A problem is that by making replace request originate from the old_vault, we can't supply a btc address as argument. Instead, we'll have to generate one like we do for issue, which does mean the client needs changes as well.

• remove the existing request_replace and withdraw_replace extrinsics

• Rename the current accept_replace to request_replace

• Change the new request_replace such that it is called by the old_vault rather than new_vault

• remove the collateral argument - instead we'll determine the collateral amount based on the amount_btc

• Remove btc_address - we'll have to auto-generate a deposit address like we do for issue

• remove replace_collateral from the vault type, it's no longer used

• On the client side, we'll have to scan for new replace deposit addresses the same way we do for issue

It's quite an involved change.. I'd like to have a think about if we can re-use the issue/redeem logic, as essentially the old-vault is making an issue with the new_vault, while the new_vault is making a redeem with the old_vault

[gregdhill]

We can't supply a btc address as argument

Why not? Do you mean from a UX perspective? In that case we can just use the master key which is already in the Vault's wallet.

Otherwise the change is as you describe, basically refactor accept_replace to request_replace. I would be happy for @nakul1010 to make that change already and then we can review after if it is possible to refactor further.

[sander2]

Why not? Do you mean from a UX perspective? In that case we can just use the master key which is already in the Vault's wallet.

The master key is per account_id, not per vault_id though, so it's not part of the vault's currency-specific wallet

[gregdhill]

Had a discussion with @sander2, we believe the best way forward is to re-use the issue and redeem logic. @nakul1010 hold-off on the implementation for now until we've had a chance to review the design.

[gregdhill]

We will refactor the replace pallet to reuse issue and redeem events / structs. Let's assume Vault A (old_vault) now wants to move their BTC to Vault B (new_vault) we can do the following:

• Vault A calls request_replace against Vault B
• Issue generates BtcAddress for Vault B
• Redeem request is generated for Vault A to send BTC to Vault B (using generated address)
  • Don't reserve wrapped tokens (since Vault A doesn't have any)
• Vault A sends BTC to Vault B (address + OP_RETURN)
• Vault A calls execute_replace with BTC transaction
  • We need to disallow execute_issue and execute_redeem
• Redeem request is executed
• Issue request is completed
  • Don't mint wrapped tokens

Some very rough code as a guide:

#[pallet::storage]
pub(super) type Requests<T: Config> = StorageMap<_, Blake2_128Concat, H256, H256>;

fn request_replace(
  old_vault_id: DefaultVaultId<T>,
  new_vault_id: DefaultVaultId<T>,
  amount: BalanceOf<T>,
) {
  // Vault B increases to_be_issued
  let (issue_id, btc_address) = Issue::request_issue(
    new_vault_id, // account to receive BTC
    amount, // amount of BTC
  );
  // Vault A increases to_be_redeemed
  let redeem_id = Redeem::request_redeem(
    old_vault_id, // account to send BTC
    btc_address, // owned by Vault B
    amount, // amount of BTC
  );
  Requests::insert(redeem_id, issue_id);
}

fn execute_replace(
  redeem_id: H256,
  unchecked_transaction: FullTransactionProof,
) {
  // Vault A decreases to_be_redeemed (and issued_tokens)
  Redeem::execute_redeem(redeem_id, unchecked_transaction);
  let issue_id = Requests::take(redeem_id);
  // Vault B decreases to_be_issued
  // Vault B increases issued_tokens
  Issue::complete_issue(issue_id);
}

Both extrinsics are submitted by the old_vault.

We should hence remove any unused replace code including storage items, events and extrinsics. We can also remove to_be_replaced_tokens, replace_collateral, and active_replace_collateral on the Vault struct. Please also consider any necessary migrations.

To disallow the other "execute" extrinsics (and generally make merging this into issue / redeem easier) it might be worth considering altering the IssueRequest and RedeemRequest structs, for example requester / redeemer could become:

enum AccountOrVault<AccountId, CurrencyId> {
  Account(AccountId),
  Vault(VaultId<AccountId, CurrencyId>)
}

[sander2]

Hmm I'm not completely convinced by this approach - both on the parachain side and on the client side this needs special handling. If we go with the issue/redeem approach, then I think for the vault client they should appear as normal issue/redeems. So the old_vault would execute by calling execute_redeem rather than execute_replace. The parachain can then apply replace logic if the AccountOrVault type is a Vault

[sander2]

Otherwise if we're ok with special handling on vault side then we might as well keep the current approach of having a special replace type, since arguably that keeps the parachain simpler

[gregdhill]

We had a call to discuss the approach and I think we are in agreement. Let's still remove the replace pallet (and Vault struct fields) but modify the following structs:

pub struct IssueRequest {
  ...
  pub requester: AccountOrVault,
}

pub struct RedeemRequest {
  ...
  pub redeemer: AccountOrVault,
  pub issue_id: Option<H256>,
}

We can add the request_replace extrinsic to the redeem pallet, but we should modify execute_redeem to include an additional code path for when issue_id is set. As stated before we should disallow the normal execution if the requester or redeemer is a Vault. Note that this approach creates an issue dependency on the redeem crate.

[sander2]

Yup agreed, just a small note that we don't need the pub redeemer: AccountOrVault, change - the request already contains the vaultid so if issue_id is set, you can just use that one