cosmos-sdk icon indicating copy to clipboard operation
cosmos-sdk copied to clipboard

Add claw back to vesting module

Open musalbas opened this issue 3 years ago • 5 comments

Summary

Add the ability for an admin key to claw back unvested tokens to a treasury account.

Problem Definition

It is standard practice in token vesting contracts, that if a team member leaves the project, the project treasury has a call option on the team member's unvested tokens, to re-purchase them at the same price they paid for them (which could be nil if they were given to the team members for no consideration).

It seems that in the current vesting module (https://docs.cosmos.network/master/modules/auth/05_vesting.html), there does not appear to be any functionality to claw back vested tokens. This therefore seems to more of a lockup module, rather than a vesting module.

Proposal

Add a function called ClawBack(treasury_address) or similar, where if it's called by the right account or signatory, unvested tokens would be sent to a treasury address. For delegated tokens that are unvested, they could be forced to be undelegated by the signatory with a function such as ForceUndelegate().


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned

musalbas avatar Nov 03 '21 17:11 musalbas

btw yearn has a similar feature: https://github.com/banteg/yearn-vesting-escrow/ specifically:

An ability to terminate an escrow and clawback all the unvested tokens using rug_pull. The recipient is still entitled to the vested portion.

As mentioned above, it might be a bit more nuanced here as the unvested tokens might be delegated. So additionally, the treasury_address or similar must be able to unbond tokens accordingly (and then claw back).

liamsi avatar Nov 03 '21 17:11 liamsi

this makes sense. Having this be optional seems like the best middle ground

tac0turtle avatar Nov 16 '21 10:11 tac0turtle

This isn't going to be trivial with the way vesting account currently work, specifically regards to unvested tokens that are delegated. Also, how does the state machine know the vesting account is owned by another account? Via the x/authz module? How will this interplay work?

alexanderbez avatar Nov 16 '21 14:11 alexanderbez

Would it make sense to upstream this implementation? https://github.com/agoric-labs/cosmos-sdk/pull/155

liamsi avatar Jan 15 '22 01:01 liamsi

In the agoric-labs#155 PR, I addressed the issue of knowing the funder by creating a new vesting account type (a variant of PeriodicVestingAccount) with the funder address as an explicit field. The clawback command is unprivileged, but must be signed by the funder.

JimLarson avatar Jan 17 '22 07:01 JimLarson

this is not super easy with the new design of accounts. should we make all accounts have a clawback call or is that too much?

tac0turtle avatar Mar 15 '24 10:03 tac0turtle

It should be optional - there are legal arrangement that require vesting but don't give the grantor the right to claw back.

There are two other features that make sense:

  • Merging additional later grants of the same type into the account. We modified PeriodicVestingAccount to have this option, as well as Agoric's ClawbackVestingAccount.
  • Giving the grantee the power to return the grant to the grantor, or at least the encumbered portion of it. We only have this for ClawbackVestingAccount, but in principle you could have it for any account type as long as you track the grantor address, and require that all additional grants (above) come from the same source.

We (Agoric) also have a change to x/staking to allow the immediate transfer of staked and unbonding tokens.

We still have plans to implement vesting grants as a concept independent of account type, so one could mix and match grants of different schedule types from different sources, as well as other extensible dimensions of encumbrance, but this won't happen for several quarters.

JimLarson avatar Mar 15 '24 16:03 JimLarson

there are legal arrangement that require vesting but don't give the grantor the right to claw back

I believe this would be called a lockup, not vesting, hence my confusion over the terminology in this module. But it makes sense for it to be optional so that this module can be for both lockups and vestings.

musalbas avatar Mar 15 '24 17:03 musalbas

Yes, my mistake, there are arrangements that require lockup (the encumbrance formerly known as vesting), but do not allow clawback. My point was that clawback shouldn't automatically be enabled for every now-called-lockup account.

JimLarson avatar Mar 15 '24 17:03 JimLarson

btw everything is a lockup now. we removed the nomencleture of vesting in the new design

tac0turtle avatar Mar 15 '24 17:03 tac0turtle

we wont make it automatic, but introduce a baselockup account that can be used to implement custom acounts

tac0turtle avatar Mar 18 '24 11:03 tac0turtle

after working on the implementation of this we discovered there is a wide range of potential designs here. With the new accounts module it is easy to implement custom lockup/vesting accounts. We would recommend people write their own instead of use the default in the sdk

tac0turtle avatar Jul 19 '24 12:07 tac0turtle