Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate vesting from auth, add custom vesting schedules #5040

Merged
merged 64 commits into from
Oct 10, 2019
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
40ed336
ADR 11: Generalize Gen Accounts implementation
fedekunze Sep 9, 2019
90a3882
move CLI cmd to auth
fedekunze Sep 10, 2019
8e92e02
delete genaccount module
fedekunze Sep 10, 2019
e1e4993
add some tests
fedekunze Sep 10, 2019
84f92dd
more fixes
fedekunze Sep 10, 2019
19fd7c8
register codec
fedekunze Sep 10, 2019
7085b9c
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into fe…
fedekunze Sep 10, 2019
0b6bbb5
Apply suggestions from code review
fedekunze Sep 11, 2019
e6bf7c4
address comments from review
fedekunze Sep 11, 2019
ac7aaef
Merge branch 'fedekunze/ADR-11-generalize-gen-accs' of https://github…
fedekunze Sep 11, 2019
e9d215a
remove genaccounts command
fedekunze Sep 11, 2019
5a69363
keep legacy genaccount migrations
fedekunze Sep 11, 2019
6c4b702
update genutil migration
fedekunze Sep 11, 2019
4e5e1a2
format
fedekunze Sep 11, 2019
6288793
migrate
fedekunze Sep 12, 2019
fe125bf
iterator
fedekunze Sep 12, 2019
e905853
feat: separate vesting from auth
karzak Sep 12, 2019
fb0e13d
fix: lint
karzak Sep 12, 2019
1198733
Update godoc
alexanderbez Sep 12, 2019
f07fabc
Update godoc
alexanderbez Sep 12, 2019
2979564
Remove RegisterAccountTypeCodec call from supply
alexanderbez Sep 12, 2019
3636574
Merge branch 'master' into fedekunze/ADR-11-generalize-gen-accs
alexanderbez Sep 12, 2019
da91323
re-add RegisterAccountTypeCodec
alexanderbez Sep 12, 2019
42b04ee
Update godoc
alexanderbez Sep 12, 2019
a65b323
Merge branch 'origin/fedekunze/ADR-11-generalize-gen-accs' into kd-se…
karzak Sep 12, 2019
4a53d93
apply changes from code review
karzak Sep 12, 2019
47b44d4
Merge remote-tracking branch 'origin/master' into kd-separate-vesting
karzak Sep 12, 2019
97b7a05
Merge remote-tracking branch 'origin/master' into kd-separate-vesting
karzak Sep 16, 2019
9dfd712
Merge remote-tracking branch 'origin/master' into kd-separate-vesting
karzak Sep 16, 2019
1de8470
fix: lint
karzak Sep 16, 2019
d7cf2ce
Merge remote-tracking branch 'origin/master' into kd-separate-vesting
karzak Sep 17, 2019
a69a107
feat: add periodic vesting account to spec
karzak Sep 17, 2019
ab06e07
address code review comments
karzak Sep 17, 2019
0e17f46
fix: lint
karzak Sep 17, 2019
f7b3feb
fix coin denoms in benchmark test
karzak Sep 18, 2019
385d4e4
fix validation of vesting periods
karzak Sep 22, 2019
88121bd
remove private methods from BaseVestingAccount
karzak Sep 22, 2019
076adae
Merge remote-tracking branch 'origin' into kd-separate-vesting
karzak Sep 22, 2019
2c35f8c
add simulation account generator function
karzak Sep 24, 2019
836f66d
lint
karzak Sep 24, 2019
29752ed
address review comments
karzak Sep 29, 2019
9054871
lint
karzak Sep 29, 2019
70940d0
lint
karzak Sep 29, 2019
3619dd5
address review comments
karzak Sep 30, 2019
655b0c1
address review comments
karzak Sep 30, 2019
fc06172
autogen alias file
karzak Sep 30, 2019
1f72304
Merge branch 'master' into kd-separate-vesting
fedekunze Sep 30, 2019
d78dba5
Merge branch 'master' into kd-separate-vesting
fedekunze Oct 1, 2019
c40ce3b
address review comments
karzak Oct 1, 2019
56a9220
lint spec
karzak Oct 1, 2019
e856664
address review comments
karzak Oct 3, 2019
914981b
Merge branch 'master' into kd-separate-vesting
alexanderbez Oct 3, 2019
9eebfb9
address review comments
karzak Oct 3, 2019
3f482d4
update spec to reflect changes
karzak Oct 3, 2019
f2ffb05
separate vesting codec from auth
karzak Oct 3, 2019
6b0629d
Merge branch 'master' into kd-separate-vesting
fedekunze Oct 7, 2019
4392ae1
address review comments
karzak Oct 7, 2019
3fe741e
Merge branch 'kd-separate-vesting' of github.com:Kava-Labs/cosmos-sdk…
karzak Oct 7, 2019
cc6fdd0
address review comments
karzak Oct 7, 2019
1e6c27c
update spendable coins name for vesting accounts
karzak Oct 7, 2019
432fdca
move vesting amount validation to contructor
karzak Oct 8, 2019
dd905b0
Update changelog
karzak Oct 8, 2019
e1f67c9
Merge branch 'master' into kd-separate-vesting
fedekunze Oct 8, 2019
a374f5f
Merge remote-tracking branch 'origin' into kd-separate-vesting
karzak Oct 10, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions x/auth/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ var (
NewBaseAccount = types.NewBaseAccount
ProtoBaseAccount = types.ProtoBaseAccount
NewBaseAccountWithAddress = types.NewBaseAccountWithAddress
NewBaseVestingAccount = types.NewBaseVestingAccount
NewContinuousVestingAccountRaw = types.NewContinuousVestingAccountRaw
NewContinuousVestingAccount = types.NewContinuousVestingAccount
NewDelayedVestingAccountRaw = types.NewDelayedVestingAccountRaw
NewDelayedVestingAccount = types.NewDelayedVestingAccount
NewAccountRetriever = types.NewAccountRetriever
RegisterCodec = types.RegisterCodec
RegisterAccountTypeCodec = types.RegisterAccountTypeCodec
Expand All @@ -70,6 +65,7 @@ var (
NewTxBuilder = types.NewTxBuilder
NewTxBuilderFromCLI = types.NewTxBuilderFromCLI
MakeSignature = types.MakeSignature
ValidateGenAccounts = types.ValidateGenAccounts
GetGenesisStateFromAppState = types.GetGenesisStateFromAppState

// variable aliases
Expand All @@ -86,12 +82,8 @@ var (
type (
SignatureVerificationGasConsumer = ante.SignatureVerificationGasConsumer
Account = exported.Account
VestingAccount = exported.VestingAccount
AccountKeeper = keeper.AccountKeeper
BaseAccount = types.BaseAccount
BaseVestingAccount = types.BaseVestingAccount
ContinuousVestingAccount = types.ContinuousVestingAccount
DelayedVestingAccount = types.DelayedVestingAccount
NodeQuerier = types.NodeQuerier
AccountRetriever = types.AccountRetriever
GenesisState = types.GenesisState
Expand Down
2 changes: 1 addition & 1 deletion x/auth/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const (
flagLimit = "limit"
)

// GetTxCmd returns the transaction commands for this module
// GetQueryCmd returns the transaction commands for this module
func GetQueryCmd(cdc *codec.Codec) *cobra.Command {
cmd := &cobra.Command{
Use: types.ModuleName,
Expand Down
20 changes: 0 additions & 20 deletions x/auth/exported/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,6 @@ type Account interface {
String() string
}

// VestingAccount defines an account type that vests coins via a vesting schedule.
type VestingAccount interface {
Account

// Delegation and undelegation accounting that returns the resulting base
// coins amount.
TrackDelegation(blockTime time.Time, amount sdk.Coins)
TrackUndelegation(amount sdk.Coins)

GetVestedCoins(blockTime time.Time) sdk.Coins
GetVestingCoins(blockTime time.Time) sdk.Coins

GetStartTime() int64
GetEndTime() int64

GetOriginalVesting() sdk.Coins
GetDelegatedFree() sdk.Coins
GetDelegatedVesting() sdk.Coins
}

// GenesisAccounts defines a slice of GenesisAccount objects
type GenesisAccounts []GenesisAccount

Expand Down
24 changes: 12 additions & 12 deletions x/auth/keeper/keeper_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ func BenchmarkAccountMapperGetAccountFoundWithCoins(b *testing.B) {
app, ctx := createTestApp(false)

coins := sdk.Coins{
sdk.NewCoin("LTC", sdk.NewInt(1000)),
sdk.NewCoin("BTC", sdk.NewInt(1000)),
sdk.NewCoin("ETH", sdk.NewInt(1000)),
sdk.NewCoin("XRP", sdk.NewInt(1000)),
sdk.NewCoin("BCH", sdk.NewInt(1000)),
sdk.NewCoin("EOS", sdk.NewInt(1000)),
sdk.NewCoin("ltc", sdk.NewInt(1000)),
sdk.NewCoin("btc", sdk.NewInt(1000)),
sdk.NewCoin("eth", sdk.NewInt(1000)),
sdk.NewCoin("xrp", sdk.NewInt(1000)),
sdk.NewCoin("bch", sdk.NewInt(1000)),
sdk.NewCoin("eos", sdk.NewInt(1000)),
}

// assumes b.N < 2**24
Expand Down Expand Up @@ -70,12 +70,12 @@ func BenchmarkAccountMapperSetAccountWithCoins(b *testing.B) {
app, ctx := createTestApp(false)

coins := sdk.Coins{
sdk.NewCoin("LTC", sdk.NewInt(1000)),
sdk.NewCoin("BTC", sdk.NewInt(1000)),
sdk.NewCoin("ETH", sdk.NewInt(1000)),
sdk.NewCoin("XRP", sdk.NewInt(1000)),
sdk.NewCoin("BCH", sdk.NewInt(1000)),
sdk.NewCoin("EOS", sdk.NewInt(1000)),
sdk.NewCoin("ltc", sdk.NewInt(1000)),
sdk.NewCoin("btc", sdk.NewInt(1000)),
sdk.NewCoin("eth", sdk.NewInt(1000)),
sdk.NewCoin("xrp", sdk.NewInt(1000)),
sdk.NewCoin("bch", sdk.NewInt(1000)),
sdk.NewCoin("eos", sdk.NewInt(1000)),
}

b.ResetTimer()
Expand Down
2 changes: 2 additions & 0 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth/client/rest"
"github.com/cosmos/cosmos-sdk/x/auth/simulation"
"github.com/cosmos/cosmos-sdk/x/auth/types"
vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
sim "github.com/cosmos/cosmos-sdk/x/simulation"
)

Expand All @@ -36,6 +37,7 @@ func (AppModuleBasic) Name() string {
// RegisterCodec registers the auth module's types for the given codec.
func (AppModuleBasic) RegisterCodec(cdc *codec.Codec) {
types.RegisterCodec(cdc)
vestingtypes.RegisterCodec(cdc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid requiring auth to register anything related to vesting? Vesting should be completely opt-in if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how'd would you make it opt-in then. By defining a module.go in vesting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two options I see are

  1. Define a module.go, which was rejected previously in e905853#r325162521
  2. Require vesting to call RegisterCodec outside the normal module manager pattern, as in ab06e07#r329289047

IMO, registering vesting types in auth doesn't makes sense, as a downstream consumer could build an app that doesn't want vesting, and would find the inclusion of vesting an anti-pattern. The tradeoff is that it needs to be clearly communicated that submodules have to be manually registered.

}

// DefaultGenesis returns default genesis state as raw bytes for the auth
Expand Down
5 changes: 3 additions & 2 deletions x/auth/simulation/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/x/auth/exported"
"github.com/cosmos/cosmos-sdk/x/auth/types"
vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types"
"github.com/cosmos/cosmos-sdk/x/simulation"
)

Expand Down Expand Up @@ -116,9 +117,9 @@ func RandomGenesisAccounts(simState *module.SimulationState) (genesisAccs export
}

if simState.Rand.Intn(100) < 50 {
gacc = types.NewContinuousVestingAccount(&bacc, startTime, endTime)
karzak marked this conversation as resolved.
Show resolved Hide resolved
gacc = vestingtypes.NewContinuousVestingAccount(&bacc, startTime, endTime)
} else {
gacc = types.NewDelayedVestingAccount(&bacc, endTime)
gacc = vestingtypes.NewDelayedVestingAccount(&bacc, endTime)
}
}
genesisAccs = append(genesisAccs, gacc)
Expand Down
150 changes: 138 additions & 12 deletions x/auth/spec/05_vesting.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,17 @@

## Intro and Requirements

This specification describes the vesting account implementation for the Cosmos Hub.
The requirements for this vesting account is that it should be initialized
during genesis with a starting balance `X` and a vesting end time `T`.
This specification defines the vesting account implementation that is used by the Cosmos Hub. The requirements for this vesting account is that it should be initialized during genesis with a starting balance `X` and a vesting end time `T`. A vesting account may be initialized with a vesting start time `T'` and a number of vesting periods `P`. If a vesting start time is included, the vesting period will not begin until start time is reached. If vesting periods are included, the vesting will occur over the specified number of periods.
karzak marked this conversation as resolved.
Show resolved Hide resolved
karzak marked this conversation as resolved.
Show resolved Hide resolved

The owner of this account should be able to delegate to and undelegate from
validators, however they cannot send locked coins to other accounts until those
coins have been fully vested.
For all vesting accounts, the owner of the vesting account is able to delegate and undelegate from validators, however they cannot transfer coins to another account until those coins are vested. This specification allows for three different kinds of vesting:
karzak marked this conversation as resolved.
Show resolved Hide resolved
* Delayed vesting, where all coins are vested once `T` is reached.
* Continous vesting, where coins begin to vest at `T'` and vest linearly with respect to time until `T` is reached
* Periodic vesting, where coins begin to vest at `T'` and vest periodically according to number of periods and the vesting amount per period. The number of periods, length per period, and amount per period are configurable. A periodic vesting account is distinguished from a continuous vesting account in that coins can be released in staggered tranches. For example, a periodic vesting account could be used for vesting arrangements where coins are relased quarterly, yearly, or over any other function of tokens over time.

In addition, a vesting account vests all of its coin denominations at the same
rate. This may be subject to change.

**Note**: A vesting account could have some vesting and non-vesting coins. To
support such a feature, the `GenesisAccount` type will need to be updated in
order to make such a distinction.
## Note
Vesting accounts can be initialized with some vesting and non-vesting coins. The non-vesting coins would be immediately transferable.
The current specification does not allow for vesting accounts to be created with normal messages after genesis. All vesting accounts must be created at genesis, or as part of a manual network upgrade.
The current specification only allows for _unconditional_ vesting (ie. there is no possibility of reaching `T` and having coins fail to vest).

## Vesting Account Types

Expand Down Expand Up @@ -83,6 +80,22 @@ type ContinuousVestingAccount struct {
type DelayedVestingAccount struct {
BaseVestingAccount
}

// VestingPeriod defines a length of time and amount of coins that will vest
type Period struct {
Length int64 // length of the period, in seconds
Amount Coins // amount of coins vesting during this period
}

// Stores all vesting periods passed as part of a PeriodicVestingAccount
type Periods []Period

// PeriodicVestingAccount implements the VestingAccount interface. It
// periodically vests by unlocking coins during each specified period
type PeriodicVestingAccount struct {
ContinuousVestingAccount
Periods Periods // the vesting schedule
}
```

In order to facilitate less ad-hoc type checking and assertions and to support
Expand Down Expand Up @@ -150,6 +163,42 @@ func (cva ContinuousVestingAccount) GetVestingCoins(t Time) Coins {
}
```

### Periodic Vesting Accounts
Periodic vesting accounts require calculating the coins released during each period for a given block time `T`. Note that multiple periods could have passed when calculating GetVestedCoins, so we must iterate over each period until the end of that period is after `T`.
karzak marked this conversation as resolved.
Show resolved Hide resolved

Set `CT := StartTime`
Set `V' := 0`
For each Period P:
1. Compute `X := T - CT`
2. IF `X >= P.Length`
a. Compute `V' += P.Amount`
b. Compute `CT += P.Length`
ELSE break
3. Compute `V := OV = V'`
karzak marked this conversation as resolved.
Show resolved Hide resolved

```go
func (pva PeriodicVestingAccount) GetVestedCoins(t Time) Coins {
if t < pva.StartTime {
return ZeroCoins
}
ct := pva.StartTime // The time of the current period start
karzak marked this conversation as resolved.
Show resolved Hide resolved
vested := 0
periods = pva.GetPeriods()
for i := 0; i < len(periods); i++ {
karzak marked this conversation as resolved.
Show resolved Hide resolved
if t - ct < periods[i].Length {
break
}
vested += periods[i].Amount
ct += periods[i].Length
}
return vested
}

func (pva PeriodicVestingAccount) GetVestingCoins(t Time) Coins {
return pva.OriginalVesting - cva.GetVestedCoins(t)
}
```

#### Delayed/Discrete Vesting Accounts

Delayed vesting accounts are easier to reason about as they only have the full
Expand Down Expand Up @@ -426,6 +475,82 @@ Same initial starting conditions as the simple example.

Notice how we have an excess amount of `DV`.

### Periodic Vesting
A vesting account is created where 100 tokens will be released over 1 year, with 1/4 of tokens vesting each quarter. The vesting schedule would be as follows:

```json
karzak marked this conversation as resolved.
Show resolved Hide resolved
{
"vesting_periods": [
{
"period_length": 7884000,
"vesting_amount": [
{
"denom": "stake",
"amount": "25"
}
]
},
{
"period_length": 7884000,
"vesting_amount": [
{
"denom": "stake",
"amount": "25"
}
]
},
{
"period_length": 7884000,
"vesting_amount": [
{
"denom": "stake",
"amount": "25"
}
]
},
{
"period_length": 7884000,
"vesting_amount": [
{
"denom": "stake",
"amount": "25"
}
]
}
]
}
```

```
OV = 100
DF = 0
DV = 0
BC = 100
V = 100
V' = 0
```

1. Immediately receives 1 coin
```
BC = 101
```
2. Vesting period 1 passes, 25 coins vest
```
V = 75
V' = 25
```
3. During vesting period 2, 5 coins are transfered and 5 coins are delegated
karzak marked this conversation as resolved.
Show resolved Hide resolved
```
DV = 4
BC = 91
```
4. Vesting period 2 passes, 25 coins vest
```
V = 50
V' = 50
```


## Glossary

- OriginalVesting: The amount of coins (per denomination) that are initially part of a vesting account. These coins are set at genesis.
Expand All @@ -435,3 +560,4 @@ Same initial starting conditions as the simple example.
- DelegatedVesting: The tracked amount of coins (per denomination) that are delegated from a vesting account that were vesting at time of delegation.
- ContinuousVestingAccount: A vesting account implementation that vests coins linearly over time.
- DelayedVestingAccount: A vesting account implementation that only fully vests all coins at a given time.
- PeriodicVestingAccount: A vesting account implementation that vests coins according to a custom vesting schedule.
Loading