cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.22k stars 3.59k forks source link

[Feature]: liquid staking UX requests #18892

Open hxrts opened 9 months ago

hxrts commented 9 months ago

Summary

changes required to make a seamless staked asset > delegation share > liquid staked asset flow

Problem Definition

if you go to stride and try to convert staked assets into liquid staked tokens you have to click several times. There are a few related problems here.

  1. Every time a new delegation voucher is created the LSM module increments a sequence number, resulting in a new asset. In order to send an asset over IBC you need to know the sequence number in advance. So in order to combine the creation of a delegation voucher and an IBC send this means a client needs to be smart and check what the current counter number is and increment by one. If someone happens to create a delegation voucher the moment before you did your transaction will fail. So there needs to be a reliable way to issue the delegation voucher and send together. One way this could be solved is allowing someone to "IBC send all delegation vouchers of any kind"
  2. This solution would also solve a second problem which is today one must know the exact amount of tokens you are sending to form an IBC transaction. If you could just "send all tokens" you wouldn't need to worry about how many tokens your delegation shares convert to, you can simply send everything you have.

Finally, there is also a related problem where transactions will fail if users assets are currently vesting. If a user could "convert all vested shares" into delegation vouchers and combine this with the IBC send all functionality described above then we could have a very clean 1 click experience with no edge cases.

Proposed Feature

The shape of a complete solution would satisfy the following function:

Convert all vested tokens into delegation shares, send all delegation shares of any kind to this IBC address

cc @zmanian

tac0turtle commented 9 months ago

Thanks for opening this issue, aggregation of tokens into a single token will be offered out of the box with the design we are thinking through.

The vesting shares seem to be ignored in the current liquid staking module. Its unclear what the decision there was since even if the vest is over the user needs to undelegate and delegate in order create shares

okwme commented 9 months ago

The vesting shares seem to be ignored in the current liquid staking module. Its unclear what the decision there was since even if the vest is over the user needs to undelegate and delegate in order create shares

just came in to confirm whether this is a bug. confusing though cause I was able to convert a lot of my vesting account delegations to delegation vouchers (update: i think this is because the delegated_free might have been updated during a hub upgrade?). I just ran into my first account that wasn't able to giving this error in the console:

desc = failed to execute message; message index: 0: trying to exceed vested free delegation for vesting account

I think this is actually a bug in the vesting logic. My account finished vesting in october 2023 and this is the time listed on the response from my account query:

{
  "@type": "/cosmos.vesting.v1beta1.ContinuousVestingAccount",
  "base_vesting_account": {
    "base_account": {
      "address": "xxxx",
      "pub_key": {
        "@type": "/cosmos.crypto.secp256k1.PubKey",
        "key": "xxx"
      },
      "account_number": "xxx",
      "sequence": "xxx"
    },
    "original_vesting": [
      {
        "denom": "uatom",
        "amount": "450000000"
      }
    ],
    "delegated_free": [
      {
        "denom": "uatom",
        "amount": "143635863"
      }
    ],
    "delegated_vesting": [
      {
        "denom": "uatom",
        "amount": "337364137"
      }
    ],
    "end_time": "1696111199"
  },
  "start_time": "1663343068"
}

As you can see the end_time is passed by 3 months but the delegated_free amount is still much less than the original_vesting. Why would that be? I'm only able to liquid stake the delegated_free amount atm.

what's also confusing is that the delegated_vesting + delegated_free is equal to exactly 481 atom instead of 450 atom. Seems unlikely that I earned exactly 31 atom in rewards, no decimals there.

I am however able to execute a successful "undelegate" tx on the remaining delegated_vesting tokens. So maybe the vesting account incorrectly calculates the delegated_free but the undelegate tx skips the invalid logic, whereas the liquid staking module listens to the invalid logic?

UPDATE: i read it wrong and closed https://github.com/cosmos/cosmos-sdk/pull/18925. Bonding or unbonding only updates the amount added or removed from delegated_free or delegated_vesting. The only way to make LST from vesting account tokens is to unbond completely. There may be something with a hub upgrade that forces a recalculation in genesis but I'm not sure. Would be really great to find a fix for this, the whole point of LST is that you don't need to unbond.

~OK I see the issue. delegated_free only gets updated after a successful unbonding occurs. The liquid staking logic only looks at the last calculated delegated_free amount here: https://github.com/cosmos/cosmos-sdk/blob/0af2f4da004cbea6414a8bad56e8bdcd45badf1e/x/staking/keeper/msg_server.go#L699-L700~

~It should run the calculation itself before checking like here~

func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins)

~example fix here: https://github.com/cosmos/cosmos-sdk/pull/18925~

~update: also confused why TrackDelegation doesn't update the vesting amounts when making a new delegation...~

alexanderbez commented 9 months ago

Yes @okwme -- we need to refactor vesting to allow for LST vouchers, specifically for the delegated_vesting portion. However, given that we're refactoring vesting and auth entirely, I'm not sure how much time we want to spend on this.

Alternatively, we can modify the code on release branch(es) such as 0.47 and 0.50, but this would be state machine breaking, so chains would have to use a fork of these.

okwme commented 8 months ago

good point @alexanderbez
I moved the issue to the gaia repo since the points you bring up are really product timeline/prioritization questions for gaia itself: https://github.com/cosmos/gaia/issues/2877

I don't think there are any other chains using the LST branch right?

alexanderbez commented 8 months ago

I don't think there are any other chains using the LST branch right?

I don't believe so, no.

robert-zaremba commented 8 months ago

I think it's NOT OK to liquid stake delegated_vesting tokens. Those tokens can't change an owner, and the idea is that they the holder doesn't have full ownership until the tokens are vested (so should not make them liquid / transfer etc...).

okwme commented 8 months ago

@robert-zaremba we're not talking about converting still vesting tokens. I'm pointing out that already vested tokens are not being calculated properly when attempting to convert them to staking vouchers.

alexanderbez commented 8 months ago

i.e. delegated_free tokens should be able to be liquid staked

sainoe commented 7 months ago

This PR addresses the LSM vesting issue.

cc @hxrts