confio / tfi

Contracts for Regulated DeFi on the Tgrade blockchain
Apache License 2.0
9 stars 0 forks source link

Allow AMM to "RedeemFor" and not just withdraw #14

Open ethanfrey opened 3 years ago

ethanfrey commented 3 years ago

Builds on #12

If I get kicked out of the DSO, I can still redeem any tokens I hold. If my tokens were in a LiquidityPool, I can no longer withdraw them, as the LP cannot Transfer the tokens to me. Let's solve this by allowing the LiquidityPool to Redeem them "in my name".

Since the essential item for redeeming is the code and not the sender, we just need to have an analogue to Withdraw on the tfi-pair contract that dispatches ExecuteMsg::Redeem{} to the asset contract, rather than ExecuteMsg::Send{} (which may fail due to whitelisting. The user must provide the code and memo field when asking the tfi-pair contract to redeem.

Note, that withdraw will provide 2 assets and this may cause issues... what if only one is a dso-token and the other native or cw20-base, which don't support redeem? What if they are both dso-tokens with different issuers and different codes needed to properly claim them?

Native assets cannot be redeemed and are always sent back to the liquidity provider (just like in Withdraw). If there is only one cw20 token involved, the user can use Redeem normally and the info will be applied to it. Todo: If there are 2 cw20 tokens??? This case needs to be defined better

Let's define the ExecuteMsg format we pass to tfi-pair, once the info is available, the actual redeem process should be pretty straight forward and just echo #12. The main part is the AMM knowing which token to redeem and how.

ethanfrey commented 3 years ago

Currently, the way to withdraw liquidity is to use Cw20HookMsg::Withdraw{} (source)

We should extend this to allow Redeem. I would propose the following format that may accept 1 or 2 redeem codes.

pub enum Cw20HookMsg {
   WithdrawAndRedeem {
      // validate length to be 1 or 2. if 2, they must have different token fields
      redeem: Vec<RedeemInfo>,
   },
}

pub struct RedeemInfo {
    // which cw20 token we wish to use this code on, it must be one of the two assets on the contract
    pub token: Addr,
    // this data will be sent along with the `Redeem` request on the token (rather than `Send` for Withdraw)
    pub code: String,
    pub memo: Option<String>,
}
ethanfrey commented 3 years ago

I am open for other proposals. This may be a bit awkward, but should contain all needed info.

hashedone commented 3 years ago

If there are two tokens, then there is no problem - your proposal basically fixes it. I have completely different issue, the same commented on #12 - what if someone reedemed tokens with invalid code(s)? Here it is even more difficult issue as there are more entities involved - I guess to solve it in similar way, acceptance from both financial institutions should be needed before actual payouts happen (and only when both accepted some event is propagated, which should trigger payout - possibly additionally storing info of accepted reedems to be payed out so banks may query them). On #12 I focused on intentional reedeming tokens without agreement which at the end is unlikely to happen, but it is vulnerable even to things like mistakes, accidental reedems, even performed by automatic trading software (as effects of bugs). I imagine that such situations may cause problems.

hashedone commented 3 years ago

The issue we discussed more here is - how to address case, when owner of cw20 token would block redeem of his tokens on holder (there is no common agreement between owner of one of cw20 tokens from pair and the holder who wants to redeem). This case is possible both in case when one and both tokens are cw20 tokens. In such case there is no way to properly perform redeeming on those tokens. We discussed some solutions, and I think that what we agreed on is, that when redeeming liquidity from pair, liquidity provider picks one of three options for both tokens:

The most controversial option is 2nd one - which pair to use to swap? Basically the very pair which is redeeming is a nice candidate, but it won't solve a problem when both tokens are part of same DSO where holder is kicked off (or maybe even another DSO which holder is not part of). Also in any DSO there should be a pair to swap for native or open (non-dso) token for every dso-token, so such pair could be used. The problem is that it would need to be part of the state of token pair, basically would need to be maintained in case if such pair changes address, or whatever. My question here is - why just not let holder to provide address to such pair. This is most generic (he can swap to token he actually can use), also I think easiest to implement. The only issue might be edge case when holder points actually pair he is redeeming from. Then such "recursive" message would be executed before state is updated, so with liquidity which is in progress of being withdrawn, which may impact how many tokens should be witdrawn. However this can be solved in two ways: it can be detected and forbidden, or it can be detected and handled internally without "recursive" message on reduced poll.

My proposal is actually not to add new redeem functionality for pair, but instead extend the withdraw liquidity message to something like:

pub enum Cw20HookMsg {
  // Swap untouched

  WithdrawLiquidity {
    // Making this optional makes actually whole think backward compatible,
    // as `WithdrawMethod::Transfer { receiver: None }` is presumed for both
    // tokens. Obviously this makes sense only for cw20 tokens, native tokens
    // are always transfered to liquidity provider.
    #[serde(default)]
    methods: Vec<WithdrawMethod>,
  }
}

pub WithdrawMethodInfo {
  // Token which this method relates to
  token: Addr,
  // Actual method to be used
  method: WithdrawMethod,
}

pub enum WithdrawMethod {
  // Just transfer tokens to given address. If no address is provided,
  // tokens are send to sender, otherwise to provided address.
  // It would fail if address is not part of dso (as dso-token would
  // reject this).
  Transfer { receiver: Option<Addr> },
  // Redeem tokens on senders behalf. Redeem code have to be
  // provided, sender is always send to original redeem sender, and
  // memo is standard generated like "Withdrawal of ... liquidity".
  // Adding optional memo to overwrite/extend generated memo is an
  // option
  Redeem { code: String },
  // Normally withdraw, but before sending tokens back, swap them
  // using provided pair contract. If none, the contract performing
  // redeem would be used (also case for setting `contract` to its
  // own addres would be detected and properly handled).
  Swap { contract: Option<Addr> },
}