Refactor vesting into general encumbrances on accounts

[JimLarson]

TL;DR

Vesting accounts are currently limited (#9959) and unsatisfactory (#9958). A plan to allow vesting grants to be added to any account can be generalized to allow modules to add encumbrances to accounts. This would be an elegant way to upstream several new features added in the Agoric chain (Agoric/agoric-sdk#4453, Agoric/agoric-sdk#4455), and if applied to staking, could simplify and regularize the bank-staking-vesting interaction.

Background

Terminology: an "encumbrance" on an account is a mechanism that prevents tokens from being transferred out of the account.

Currently, there are two uses of encumbrance: unvested coins in vesting accounts, and bonded or unbonding coins in staking. Staking, being the original encumbrance, has been implemented by simply transfering the coins out of the account, then transfering the unslashed remainder back when unbonded - but the rest of the system has to bend over backward to pretend that the coins are still in the account (e.g. wallet software must add the staked amount to the bank balance to display total balance). Vesting accounts need an account-specific hook into the bank module to let the bank query the number of unvested (locked) coins, plus they require the bank module to track staking to update the bookkeeping stored in the vesting account[1], to manage the custom interaction between vesting and staking.

From a sufficiently high level of abstraction, there is a single mechanism at work. There are two "dimensions" of encumbrance: "staking" and "lockup"[2]. The amount actually encumbered in the account is the maximum of the amount in the two dimensions (e.g. if 300stake is staked, and 200stake and 50stable are unvested, then 300stake and 50stable would not be able to be transferred out of the account). Thus, the system does not require you to stake a particular amount of locked vs unlocked tokens, and vesting events do not unlock a certain number of staked vs unstaked tokens, but instead each dimension behaves independently, and the system maximizes the number of freely-transferrable tokens by overlapping the restrictions from the separate dimensions as much as possible.

Because of the combination of separate dimensions, we'll call them "disjunctive liabilities" on the account.

Problem

In the Agoric fork of the sdk (Agoric/cosmos-sdk), we've implemented two new features which fit into this framework:

• The ability to put a lien against a certain number of tokens in an account, preventing them from being transferred out of the account until the lien is lifted (to be used to obtain an advance on future staking rewards).
• Clawback vesting accounts, which have independent tracking of a locked amount which is not subject to clawback, and an unvested amount which is subject to clawback.

Since we don't allow a lien to be taken on unvested (in the clawback sense) tokens, these two features occupy a single dimension of encumbrance.

The Agoric fork implements clawback through internal logic in the ClawbackVestingAccount implementation.

Liens are more complicated. Since liens can be taken on any account type, those require some slick wrapping of AuthKeeper to automatically wrap and unwrap accounts going to and from storage to make everything act like a VestingAccount account, triggering queries of the locked amount which are served by our lien module.

We'd obviously like to do this in a better way.

Proposed Solution

We propose a new "disjunctive liabilities" module (or alternatively, add this as a new feature to the x/bank module) that would work as follows:

• The application can configure a set of labeled dimensions of encumbrance.
• The amount encumbered along each dimension would be computed via callouts installed by other modules, with a signature something like type Liability interface { LockedCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins }. Multiple callbacks registered on the same dimension add together to get the total locked amount for that dimension.
• The disjunctive liabilities module provides a total LockedCoins() interface to the x/bank module which gives the maximum of the amount locked among all the dimensions (per denom, see Coins.Max()).
• The bank module calls this LockedCoins() for all accounts to compute the spendable amount, replacing the current hook that is used for vesting accounts.
• The various kinds of vesting accounts can be replaced by equivalent kinds of vesting grants. Any account can receive a grant, or multiple grants of the same or different types. The total amount encumbered is the sum of the locked amount of each grant.
• The staking module would track the net amount staked for all accounts, not just for vesting accounts.

Optionally, the mechanism could even embrace staking! Staked tokens would not be transferred into a separate module account. Instead, a liability would be put against them in the a "staking" dimension, orthogonal from any other liability dimension. All the other staking machinery would be the same, but instead of transferring tokens in and out of the module accounts, the corresponding liabilities would be adjusted to encumber or unencumber the tokens. This way, the simple bank balance query would yield the total account value, and the total staked amount (modulo slashing) would be visible from the current staking dimension value.

(Slashing would be slightly changed - instead of immediately burning the tokens from the slashing module account, they would be burned on a per-account basis when the slashing was recognized. The end-user experience would be mostly the same.)

With staking as just another kind of liability, the staking tracking currently done by vesting accounts is now tracked and published by the staking module itself, rather than requiring the bank module to do so.

Migration

Migration would perform the following tasks:

• Vesting accounts would translate their vesting information into vesting grants, with the residual account information translated into a default normal user account type.
• The staking module would compute and populate the total amount staked for each account with nonzero staking (extended scenario only).
• Application setup would need to install the appropriate callbacks for the disjunctive liabilities module, and help the bank module find it.

Other Issues

There would need to be a coherent plan for the current ecosystem of tools which expect the current interfaces of staking and vesting.

We would need to prevent "spamming" of existing accounts by giving them obnoxious vesting grants, e.g. with thousands of vesting events. The change suggested below would reduce locked amount lookup to O(log N) time. It might be necessary to limit the size of vesting schedule or charge a fee proportional to their size.

Notes on Vesting Accounts

As long as we're changing the representation of vesting accounts, it would be a good time to change PeriodicVestingAccount (and ClawbackVestingAccount) to have a schedule based on absolute timestamps giving total locked amount rather than a list of time intervals and unlocking increments. The new representation would allow fast lookup via binary search rather than summing the schedule.

Allowing additional vesting grants results in a subtle interaction with slashing. Currently, the DV + DF tracking makes slashing look like tokens that are forever staked. Suppose that I have a vesting account where I staked all my unvested tokens, got slashed for 100stake, and now everything is fully vested an unstaked. The account remains with DV=100stake. Suppose I then get a grant of 200stake which is supposed to remain locked for 12 months. But because of the leftover bookkeeping, half of this new grant falls into the "hole" leftover from the old bookkeeping, and 100 of the tokens which were transferred in with the grant become
immediately available. The solution is that when adding a new vesting grant to an account we must recognize any slashed amount and adjust the bookkeeping to "write it off".

[1] It appears that the DV and DF accounting could be simplified for vesting accounts - it is only necessary to track the current net amount staked: the sum of all historical delegation minus the sum of all historical undelegation. Slashing manifests as an amount that is staked and never unstaked.

[2] The term "vesting" as currently used by cosmos-sdk does not match the legal meaning of the term, primarily in the sense that it is not subject to clawback (#10662). In the Agoric extension to provide clawback, we've referred to the dimensions as "lockup" and "clawback", avoiding "vesting" as dangerously ambiguous.

[JimLarson]

Note potential overlap with #14124 ("Add ability to quarantine and sanctioned accounts."). There is interest in not providing multiple APIs that do the same thing, so will need to either subsume one in the other, or provide clear distinction.

[SpicyLemon]

Semi-relatedly, I've asked for some changes in the bank module to allow other modules to inject restrictions on sends: #14224. I'd be great if the restrictions being added in that PR cover the use cases in this discussion too.

The injectable is a function.

// A SendRestrictionFn can restrict sends and/or provide a new receiver address.
type SendRestrictionFn func(ctx sdk.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) (newToAddr sdk.AccAddress, err error)

They're injected into the bank module using one of these on the bank keeper:

AppendSendRestriction(restriction SendRestrictionFn)
PrependSendRestriction(restriction SendRestrictionFn)

They are applied in both SendCoins:

if k.sendRestriction.Fn != nil {
	var err error
	toAddr, err = k.sendRestriction.Fn(ctx, fromAddr, toAddr, amt)
	if err != nil {
		return err
	}
}

and InputOutputCoins, for each Output:

if k.sendRestriction.Fn != nil {
	outAddress, err = k.sendRestriction.Fn(ctx, inAddress, outAddress, out.Coins)
	if err != nil {
		return err
	}
}

With respect to the purposes in this discussion:

1. Are these changes to the bank module sufficient for your needs?
2. Do these changes make it more difficult to implement what's needed in the bank module?

[JimLarson]

Sorry for the late reply, @SpicyLemon. Repeating the comments I made in #14224, yes, the SendRestrictionFn you propose would happily accommodate disjunctive liabilities as a single restriction. Or each "dimension" could be its own restriction - no need to have two levels of combining effects when one will do. I suggest that you could already in your PR refactor the bank keeper's subUnlockedCoins() as a restriction check, then a simpler unconditional subCoins() action for nice symmetry with addCoins(). It could be done in a later PR, I suppose, but it would be nice to show that your mechanism can unify all existing restrictions as well as new ones.

[alexanderbez]

@JimLarson I mentioned in today's community call that @fdymylja has worked an audited vesting abstraction (in light of us trying to use BaseAccount only for all account operations).

Would love for you two to sync on common use-cases and goals for vesting. I believe @fdymylja was going to write an ADR on this soon.

[JimLarson]

Thank you, @alexanderbez ! I've reached out to @fdymylja on Discord.

[JimLarson]

I gave a presentation on this idea to WG-AUTH. slides

Possible uses of general encumbrances:

• staking (privileged dimension which gets to encumber by removing tokens);
• vesting accounts without clawback (current feature, refactored into a module instead of an account type);
• vesting accounts with clawback;
• liens to encumber debt against future staking rewards;
• committed votes to a governance action so votes can be tallied incrementally;
• demonstration of assets in lieu of fees (think of checking accounts which waive fees when a minimum balance is met).