From be182d24e037570676f13cd0d6ccccb9b3280d59 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 11:13:26 -0500 Subject: [PATCH 01/76] Implement initial vesting account interface --- x/auth/account.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 4a55a48ea3df..7b09f8c986e2 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -31,10 +31,22 @@ type Account interface { SetCoins(sdk.Coins) error } +// VestingAccount defines an account type that vests coins via a vesting schedule. +type VestingAccount interface { + Account + + // Calculates the amount of coins that can be sent to other accounts given + // the current time. + SpendableCoins(ctx sdk.Context) sdk.Coins + + TrackDelegation(amount sdk.Coins) // Performs delegation accounting. + TrackUndelegation(amount sdk.Coins) // Performs undelegation accounting. +} + // AccountDecoder unmarshals account bytes type AccountDecoder func(accountBytes []byte) (Account, error) -//----------------------------------------------------------- +//----------------------------------------------------------------------------- // BaseAccount var _ Account = (*BaseAccount)(nil) @@ -121,8 +133,8 @@ func (acc *BaseAccount) SetSequence(seq int64) error { return nil } -//---------------------------------------- -// Wire +//----------------------------------------------------------------------------- +// Codec // Most users shouldn't use this, but this comes in handy for tests. func RegisterBaseAccount(cdc *codec.Codec) { From 043f90a4bb5aca6f69cbeb331cf42fda34220bbd Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 14:04:38 -0500 Subject: [PATCH 02/76] Add vesting account types --- x/auth/account.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/x/auth/account.go b/x/auth/account.go index 7b09f8c986e2..a9f8455f8f95 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -2,6 +2,7 @@ package auth import ( "errors" + "time" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" @@ -133,12 +134,53 @@ func (acc *BaseAccount) SetSequence(seq int64) error { return nil } +//----------------------------------------------------------------------------- +// Vesting Accounts + +var ( + _ VestingAccount = (*ContinuousVestingAccount)(nil) + _ VestingAccount = (*DelayedVestingAccount)(nil) +) + +type ( + // BaseVestingAccount implements the VestingAccount interface. It contains all + // the necessary fields needed for any vesting account implementation. + BaseVestingAccount struct { + BaseAccount + + OriginalVesting sdk.Coins // coins in account upon initialization + DelegatedFree sdk.Coins // coins that are vested and delegated + EndTime time.Time // when the coins become unlocked + } + + // ContinuousVestingAccount implements the VestingAccount interface. It + // continuously vests by unlocking coins linearly with respect to time. + ContinuousVestingAccount struct { + BaseAccount + BaseVestingAccount + + DelegatedVesting sdk.Coins // coins that vesting and delegated + StartTime time.Time // when the coins start to vest + } + + // DelayedVestingAccount implements the VestingAccount interface. It vests all + // coins after a specific time, but non prior. In other words, it keeps them + // locked until a specified time. + DelayedVestingAccount struct { + BaseAccount + BaseVestingAccount + } +) + //----------------------------------------------------------------------------- // Codec // Most users shouldn't use this, but this comes in handy for tests. func RegisterBaseAccount(cdc *codec.Codec) { cdc.RegisterInterface((*Account)(nil), nil) + cdc.RegisterInterface((*VestingAccount)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/BaseAccount", nil) + cdc.RegisterConcrete(&ContinuousVestingAccount{}, "cosmos-sdk/ContinuousVestingAccount", nil) + cdc.RegisterConcrete(&DelayedVestingAccount{}, "cosmos-sdk/DelayedVestingAccount", nil) codec.RegisterCrypto(cdc) } From d01240585b8cd7dc76997e1919a5b5b6ba91a2d0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 15:38:25 -0500 Subject: [PATCH 03/76] Update vesting spec per impl updates --- docs/spec/auth/vesting.md | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index b8785619b2aa..5166666961b2 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -80,7 +80,6 @@ type BaseVestingAccount struct { // ContinuousVestingAccount implements the VestingAccount interface. It // continuously vests by unlocking coins linearly with respect to time. type ContinuousVestingAccount struct { - BaseAccount BaseVestingAccount DelegatedVesting Coins // coins that vesting and delegated @@ -91,7 +90,6 @@ type ContinuousVestingAccount struct { // coins after a specific time, but non prior. In other words, it keeps them // locked until a specified time. type DelayedVestingAccount struct { - BaseAccount BaseVestingAccount } ``` @@ -130,7 +128,7 @@ func (cva ContinuousVestingAccount) GetVestedCoins(b Block) Coins { // We must handle the case where the start time for a vesting account has // been set into the future or when the start of the chain is not exactly // known. - if b.Time < va.StartTime { + if b.Time <= va.StartTime { return ZeroCoins } @@ -175,9 +173,9 @@ balance and the base account balance plus the number of currently delegated vesting coins less the number of coins vested so far. ```go -func (cva ContinuousVestingAccount) SpendableCoins() Coins { +func (cva ContinuousVestingAccount) SpendableCoins(b Block) Coins { bc := cva.GetCoins() - return min((bc + cva.DelegatedVesting) - cva.GetVestingCoins(), bc) + return min((bc + cva.DelegatedVesting) - cva.GetVestingCoins(b), bc) } ``` @@ -187,9 +185,9 @@ A delayed vesting account may send any coins it has received. In addition, if it has fully vested, it can send any of it's vested coins. ```go -func (dva DelayedVestingAccount) SpendableCoins() Coins { +func (dva DelayedVestingAccount) SpendableCoins(b Block) Coins { bc := dva.GetCoins() - return bc - dva.GetVestingCoins() + return bc - dva.GetVestingCoins(b) } ``` From 4fbd311ed4369c6da1e9dae2b27397a834355636 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 15:39:53 -0500 Subject: [PATCH 04/76] Implement vesting/vested coins for cont. vesting account --- x/auth/account.go | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index a9f8455f8f95..2445e433666c 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -156,7 +156,6 @@ type ( // ContinuousVestingAccount implements the VestingAccount interface. It // continuously vests by unlocking coins linearly with respect to time. ContinuousVestingAccount struct { - BaseAccount BaseVestingAccount DelegatedVesting sdk.Coins // coins that vesting and delegated @@ -167,11 +166,55 @@ type ( // coins after a specific time, but non prior. In other words, it keeps them // locked until a specified time. DelayedVestingAccount struct { - BaseAccount BaseVestingAccount } ) +func NewContinuousVestingAccount( + addr sdk.AccAddress, origCoins sdk.Coins, startTime, endTime time.Time, +) ContinuousVestingAccount { + + baseAcc := NewBaseAccountWithAddress(addr) + baseVestingAcc := BaseVestingAccount{ + BaseAccount: baseAcc, + OriginalVesting: origCoins, + EndTime: endTime, + } + return ContinuousVestingAccount{BaseVestingAccount: baseVestingAcc, StartTime: startTime} +} + +// GetVestedCoins returns the total number of vested coins. If no coins are vested, +// nil is returned. +func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { + var vestedCoins sdk.Coins + + // We must handle the case where the start time for a vesting account has + // been set into the future or when the start of the chain is not exactly + // known. + if blockTime.Unix() <= cva.StartTime.Unix() { + return vestedCoins + } + + // calculate the vesting scalar + x := blockTime.Unix() - cva.StartTime.Unix() + y := cva.EndTime.Unix() - cva.StartTime.Unix() + s := sdk.NewDec(x).Quo(sdk.NewDec(y)) + + for _, ovc := range cva.OriginalVesting { + vestedAmt := sdk.NewDecFromInt(ovc.Amount).Mul(s).RoundInt() + vestedCoin := sdk.NewCoin(ovc.Denom, vestedAmt) + vestedCoins = vestedCoins.Plus(sdk.Coins{vestedCoin}) + } + + return vestedCoins +} + +// GetVestingCoins returns the total number of vesting coins. If no coins are +// vesting, nil is returned. +func (cva ContinuousVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { + return cva.OriginalVesting.Minus(cva.GetVestedCoins(blockTime)) +} + //----------------------------------------------------------------------------- // Codec From ba7e14c6a2253379e2d331963e2c31da10b4f50d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 15:40:39 -0500 Subject: [PATCH 05/76] Implement unit tests for cont. vesting account --- x/auth/account_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/x/auth/account_test.go b/x/auth/account_test.go index e48060fbefc9..ebb2a2091ae9 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -106,3 +106,45 @@ func TestBaseAccountMarshal(t *testing.T) { err = cdc.UnmarshalBinaryLengthPrefixed(b[:len(b)/2], &acc2) require.NotNil(t, err) } + +func TestGetVestedCoinsContVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) + + // require no coins vested in the very begining of the vesting schedule + vestedCoins := cva.GetVestedCoins(now) + require.Nil(t, vestedCoins) + + // require all coins vested at the end of the vesting schedule + vestedCoins = cva.GetVestedCoins(endTime) + require.Equal(t, origCoins, vestedCoins) + + // require 50% of coins vested + vestedCoins = cva.GetVestedCoins(now.Add(12 * time.Hour)) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, vestedCoins) +} + +func TestGetVestingCoinsContVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) + + // require all coins vesting in the begining of the vesting schedule + vestingCoins := cva.GetVestingCoins(now) + require.Equal(t, origCoins, vestingCoins) + + // require no coins vesting at the end of the vesting schedule + vestingCoins = cva.GetVestingCoins(endTime) + require.Nil(t, vestingCoins) + + // require 50% of coins vesting + vestingCoins = cva.GetVestingCoins(now.Add(12 * time.Hour)) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, vestingCoins) +} From 541608df1ef9042230edb915d4a73beb7ca37d6f Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 15:51:42 -0500 Subject: [PATCH 06/76] Implement SpendableCoins for cont. vesting account --- x/auth/account.go | 53 ++++++++++++++++++++++++++++++++++++++---- x/auth/account_test.go | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 2445e433666c..67e28f778411 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -2,6 +2,7 @@ package auth import ( "errors" + "fmt" "time" "github.com/cosmos/cosmos-sdk/codec" @@ -137,10 +138,11 @@ func (acc *BaseAccount) SetSequence(seq int64) error { //----------------------------------------------------------------------------- // Vesting Accounts -var ( - _ VestingAccount = (*ContinuousVestingAccount)(nil) - _ VestingAccount = (*DelayedVestingAccount)(nil) -) +// TODO: uncomment once implemented +// var ( +// _ VestingAccount = (*ContinuousVestingAccount)(nil) +// _ VestingAccount = (*DelayedVestingAccount)(nil) +// ) type ( // BaseVestingAccount implements the VestingAccount interface. It contains all @@ -175,12 +177,18 @@ func NewContinuousVestingAccount( ) ContinuousVestingAccount { baseAcc := NewBaseAccountWithAddress(addr) + baseAcc.SetCoins(origCoins) + baseVestingAcc := BaseVestingAccount{ BaseAccount: baseAcc, OriginalVesting: origCoins, EndTime: endTime, } - return ContinuousVestingAccount{BaseVestingAccount: baseVestingAcc, StartTime: startTime} + + return ContinuousVestingAccount{ + StartTime: startTime, + BaseVestingAccount: baseVestingAcc, + } } // GetVestedCoins returns the total number of vested coins. If no coins are vested, @@ -215,6 +223,41 @@ func (cva ContinuousVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coi return cva.OriginalVesting.Minus(cva.GetVestedCoins(blockTime)) } +// SpendableCoins returns the total number of spendable coins per denom for a +// continuous vesting account. +func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { + var spendableCoins sdk.Coins + + bc := cva.GetCoins() + v := cva.GetVestingCoins(blockTime) + + for _, coin := range bc { + baseAmt := coin.Amount + delVestingAmt := cva.DelegatedVesting.AmountOf(coin.Denom) + vestingAmt := v.AmountOf(coin.Denom) + + a := baseAmt.Add(delVestingAmt) + a = a.Sub(vestingAmt) + + var spendableCoin sdk.Coin + + // compute the min((baseAmt + delVestingAmt) - vestingAmt, baseAmt) + fmt.Println(a) + fmt.Println(baseAmt) + if a.LT(baseAmt) { + spendableCoin = sdk.NewCoin(coin.Denom, a) + } else { + spendableCoin = sdk.NewCoin(coin.Denom, baseAmt) + } + + if !spendableCoin.IsZero() { + spendableCoins = spendableCoins.Plus(sdk.Coins{spendableCoin}) + } + } + + return spendableCoins +} + //----------------------------------------------------------------------------- // Codec diff --git a/x/auth/account_test.go b/x/auth/account_test.go index ebb2a2091ae9..096ebd0e1f90 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -2,16 +2,22 @@ package auth import ( "testing" + "time" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" + tmtime "github.com/tendermint/tendermint/types/time" codec "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" ) +var ( + testDenom = "steak" +) + func keyPubAddr() (crypto.PrivKey, crypto.PubKey, sdk.AccAddress) { key := ed25519.GenPrivKey() pub := key.PubKey() @@ -148,3 +154,41 @@ func TestGetVestingCoinsContVestingAcc(t *testing.T) { vestingCoins = cva.GetVestingCoins(now.Add(12 * time.Hour)) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, vestingCoins) } + +func TestSpendableCoinsContVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) + + // require that there exist no spendable coins in the beginning of the + // vesting schedule + spendableCoins := cva.SpendableCoins(now) + require.Nil(t, spendableCoins) + + // require that all original coins are spendable at the end of the vesting + // schedule + spendableCoins = cva.SpendableCoins(endTime) + require.Equal(t, origCoins, spendableCoins) + + // require that all vested coins (50%) are spendable + spendableCoins = cva.SpendableCoins(now.Add(12 * time.Hour)) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, spendableCoins) + + // receive some coins + recvAmt := sdk.Coins{sdk.NewInt64Coin(testDenom, 50)} + cva.SetCoins(cva.GetCoins().Plus(recvAmt)) + + // require that all vested coins (50%) are spendable plus any received + spendableCoins = cva.SpendableCoins(now.Add(12 * time.Hour)) + require.Equal(t, origCoins, spendableCoins) + + // spend all spendable coins + cva.SetCoins(cva.GetCoins().Minus(spendableCoins)) + + // require that no more coins are spendable + spendableCoins = cva.SpendableCoins(now.Add(12 * time.Hour)) + require.Nil(t, spendableCoins) +} From e21acac504fc02394d39e08bfe6a8f1ee76e47bc Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 16:04:28 -0500 Subject: [PATCH 07/76] Update spec variables and parameters --- docs/spec/auth/vesting.md | 49 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 5166666961b2..a16f8393d1a8 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -56,15 +56,14 @@ order to make such a distinction. // implement. type VestingAccount interface { Account - AssertIsVestingAccount() // existence implies that account is vesting // Calculates the amount of coins that can be sent to other accounts given // the current time. SpendableCoins(Context) Coins // Performs delegation accounting. - TrackDelegation(amount) + TrackDelegation(Time, Coins) // Performs undelegation accounting. - TrackUndelegation(amount) + TrackUndelegation(Coins) } // BaseVestingAccount implements the VestingAccount interface. It contains all @@ -112,10 +111,10 @@ mandatory per-block basis. #### Continuously Vesting Accounts -To determine the amount of coins that are vested for a given block `B`, the +To determine the amount of coins that are vested for a given block time `T`, the following is performed: -1. Compute `X := B.Time - StartTime` +1. Compute `X := T - StartTime` 2. Compute `Y := EndTime - StartTime` 3. Compute `V' := OV * (X / Y)` 4. Compute `V := OV - V'` @@ -124,22 +123,22 @@ Thus, the total amount of _vested_ coins is `V'` and the remaining amount, `V`, is _vesting_. ```go -func (cva ContinuousVestingAccount) GetVestedCoins(b Block) Coins { +func (cva ContinuousVestingAccount) GetVestedCoins(t Time) Coins { // We must handle the case where the start time for a vesting account has // been set into the future or when the start of the chain is not exactly // known. - if b.Time <= va.StartTime { + if t <= va.StartTime { return ZeroCoins } - x := b.Time - cva.StartTime + x := t - cva.StartTime y := cva.EndTime - cva.StartTime return cva.OriginalVesting * (x / y) } -func (cva ContinuousVestingAccount) GetVestingCoins(b Block) Coins { - return cva.OriginalVesting - cva.GetVestedCoins(b) +func (cva ContinuousVestingAccount) GetVestingCoins(t Time) Coins { + return cva.OriginalVesting - cva.GetVestedCoins(t) } ``` @@ -149,16 +148,16 @@ Delayed vesting accounts are easier to reason about as they only have the full amount vesting up until a certain time, then they all become vested (unlocked). ```go -func (dva DelayedVestingAccount) GetVestedCoins(b Block) Coins { - if b.Time >= dva.EndTime { +func (dva DelayedVestingAccount) GetVestedCoins(t Time) Coins { + if t >= dva.EndTime { return dva.OriginalVesting } return ZeroCoins } -func (dva DelayedVestingAccount) GetVestingCoins(b Block) Coins { - return cva.OriginalVesting - cva.GetVestedCoins(b) +func (dva DelayedVestingAccount) GetVestingCoins(t Time) Coins { + return cva.OriginalVesting - cva.GetVestedCoins(t) } ``` @@ -173,9 +172,9 @@ balance and the base account balance plus the number of currently delegated vesting coins less the number of coins vested so far. ```go -func (cva ContinuousVestingAccount) SpendableCoins(b Block) Coins { +func (cva ContinuousVestingAccount) SpendableCoins(t Time) Coins { bc := cva.GetCoins() - return min((bc + cva.DelegatedVesting) - cva.GetVestingCoins(b), bc) + return min((bc + cva.DelegatedVesting) - cva.GetVestingCoins(t), bc) } ``` @@ -185,9 +184,9 @@ A delayed vesting account may send any coins it has received. In addition, if it has fully vested, it can send any of it's vested coins. ```go -func (dva DelayedVestingAccount) SpendableCoins(b Block) Coins { +func (dva DelayedVestingAccount) SpendableCoins(t Time) Coins { bc := dva.GetCoins() - return bc - dva.GetVestingCoins(b) + return bc - dva.GetVestingCoins(t) } ``` @@ -197,9 +196,9 @@ The corresponding `x/bank` keeper should appropriately handle sending coins based on if the account is a vesting account or not. ```go -func SendCoins(from Account, to Account amount Coins) { +func SendCoins(t Time, from Account, to Account, amount Coins) { if isVesting(from) { - sc := from.SpendableCoins() + sc := from.SpendableCoins(t) } else { sc := from.GetCoins() } @@ -227,8 +226,8 @@ is performed: 6. Set `BC -= D` ```go -func (cva ContinuousVestingAccount) TrackDelegation(amount Coins) { - x := min(max(cva.GetVestingCoins() - cva.DelegatedVesting, 0), amount) +func (cva ContinuousVestingAccount) TrackDelegation(t Time, amount Coins) { + x := min(max(cva.GetVestingCoins(t) - cva.DelegatedVesting, 0), amount) y := amount - x cva.DelegatedVesting += x @@ -242,7 +241,7 @@ For a delayed vesting account, it can only delegate with received coins and coins that are fully vested so we only need to update `DF`. ```go -func (dva DelayedVestingAccount) TrackDelegation(amount Coins) { +func (dva DelayedVestingAccount) TrackDelegation(t Time, amount Coins) { dva.DelegatedFree += amount } ``` @@ -250,14 +249,14 @@ func (dva DelayedVestingAccount) TrackDelegation(amount Coins) { ##### Keepers/Handlers ```go -func DelegateCoins(from Account, amount Coins) { +func DelegateCoins(t Time, from Account, amount Coins) { // canDelegate checks different semantics for continuous and delayed vesting // accounts if isVesting(from) && canDelegate(from) { sc := from.GetCoins() if amount <= sc { - from.TrackDelegation(amount) + from.TrackDelegation(t, amount) from.SetCoins(sc - amount) // save account... } From 75a6673fdb04bb2b74d43e07fcc36ee2a9b051f0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 16:19:38 -0500 Subject: [PATCH 08/76] Implement max int and max uint --- types/int.go | 19 +++++++++++++++++++ types/int_test.go | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/types/int.go b/types/int.go index c2bef7a64fcb..6afcafba62d4 100644 --- a/types/int.go +++ b/types/int.go @@ -36,6 +36,15 @@ func min(i *big.Int, i2 *big.Int) *big.Int { if i.Cmp(i2) == 1 { return new(big.Int).Set(i2) } + + return new(big.Int).Set(i) +} + +func max(i *big.Int, i2 *big.Int) *big.Int { + if i.Cmp(i2) == -1 { + return new(big.Int).Set(i2) + } + return new(big.Int).Set(i) } @@ -258,6 +267,11 @@ func MinInt(i1, i2 Int) Int { return Int{min(i1.BigInt(), i2.BigInt())} } +// MaxInt returns the maximum between two integers. +func MaxInt(i, i2 Int) Int { + return Int{max(i.BigInt(), i2.BigInt())} +} + // Human readable string func (i Int) String() string { return i.i.String() @@ -485,6 +499,11 @@ func MinUint(i1, i2 Uint) Uint { return Uint{min(i1.BigInt(), i2.BigInt())} } +// MaxUint returns the maximum between two unsigned integers. +func MaxUint(i, i2 Uint) Uint { + return Uint{max(i.BigInt(), i2.BigInt())} +} + // Human readable string func (i Uint) String() string { return i.i.String() diff --git a/types/int_test.go b/types/int_test.go index cd357c4f77ac..42f279bf0ab5 100644 --- a/types/int_test.go +++ b/types/int_test.go @@ -145,6 +145,13 @@ func minint(i1, i2 int64) int64 { return i2 } +func maxint(i1, i2 int64) int64 { + if i1 > i2 { + return i1 + } + return i2 +} + func TestArithInt(t *testing.T) { for d := 0; d < 1000; d++ { n1 := int64(rand.Int31()) @@ -165,6 +172,7 @@ func TestArithInt(t *testing.T) { {i1.MulRaw(n2), n1 * n2}, {i1.DivRaw(n2), n1 / n2}, {MinInt(i1, i2), minint(n1, n2)}, + {MaxInt(i1, i2), maxint(n1, n2)}, {i1.Neg(), -n1}, } @@ -226,6 +234,13 @@ func minuint(i1, i2 uint64) uint64 { return i2 } +func maxuint(i1, i2 uint64) uint64 { + if i1 > i2 { + return i1 + } + return i2 +} + func TestArithUint(t *testing.T) { for d := 0; d < 1000; d++ { n1 := uint64(rand.Uint32()) @@ -244,6 +259,7 @@ func TestArithUint(t *testing.T) { {i1.MulRaw(n2), n1 * n2}, {i1.DivRaw(n2), n1 / n2}, {MinUint(i1, i2), minuint(n1, n2)}, + {MaxUint(i1, i2), maxuint(n1, n2)}, } for tcnum, tc := range cases { From 149a1b624fbfa5effbf0553be6765bcd4c186f94 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 5 Nov 2018 16:54:44 -0500 Subject: [PATCH 09/76] Track delegation cont. vesting account implementation --- x/auth/account.go | 87 ++++++++++++++++++++++++++---------------- x/auth/account_test.go | 36 +++++++++++++++++ 2 files changed, 91 insertions(+), 32 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 67e28f778411..85b7e033b43e 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -2,7 +2,6 @@ package auth import ( "errors" - "fmt" "time" "github.com/cosmos/cosmos-sdk/codec" @@ -41,8 +40,8 @@ type VestingAccount interface { // the current time. SpendableCoins(ctx sdk.Context) sdk.Coins - TrackDelegation(amount sdk.Coins) // Performs delegation accounting. - TrackUndelegation(amount sdk.Coins) // Performs undelegation accounting. + TrackDelegation(blockTime time.Time, amount sdk.Coins) // Performs delegation accounting. + TrackUndelegation(amount sdk.Coins) // Performs undelegation accounting. } // AccountDecoder unmarshals account bytes @@ -150,9 +149,9 @@ type ( BaseVestingAccount struct { BaseAccount - OriginalVesting sdk.Coins // coins in account upon initialization - DelegatedFree sdk.Coins // coins that are vested and delegated - EndTime time.Time // when the coins become unlocked + originalVesting sdk.Coins // coins in account upon initialization + delegatedFree sdk.Coins // coins that are vested and delegated + endTime time.Time // when the coins become unlocked } // ContinuousVestingAccount implements the VestingAccount interface. It @@ -160,8 +159,8 @@ type ( ContinuousVestingAccount struct { BaseVestingAccount - DelegatedVesting sdk.Coins // coins that vesting and delegated - StartTime time.Time // when the coins start to vest + delegatedVesting sdk.Coins // coins that vesting and delegated + startTime time.Time // when the coins start to vest } // DelayedVestingAccount implements the VestingAccount interface. It vests all @@ -174,19 +173,19 @@ type ( func NewContinuousVestingAccount( addr sdk.AccAddress, origCoins sdk.Coins, startTime, endTime time.Time, -) ContinuousVestingAccount { +) *ContinuousVestingAccount { baseAcc := NewBaseAccountWithAddress(addr) baseAcc.SetCoins(origCoins) baseVestingAcc := BaseVestingAccount{ BaseAccount: baseAcc, - OriginalVesting: origCoins, - EndTime: endTime, + originalVesting: origCoins, + endTime: endTime, } - return ContinuousVestingAccount{ - StartTime: startTime, + return &ContinuousVestingAccount{ + startTime: startTime, BaseVestingAccount: baseVestingAcc, } } @@ -199,16 +198,16 @@ func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coin // We must handle the case where the start time for a vesting account has // been set into the future or when the start of the chain is not exactly // known. - if blockTime.Unix() <= cva.StartTime.Unix() { + if blockTime.Unix() <= cva.startTime.Unix() { return vestedCoins } // calculate the vesting scalar - x := blockTime.Unix() - cva.StartTime.Unix() - y := cva.EndTime.Unix() - cva.StartTime.Unix() + x := blockTime.Unix() - cva.startTime.Unix() + y := cva.endTime.Unix() - cva.startTime.Unix() s := sdk.NewDec(x).Quo(sdk.NewDec(y)) - for _, ovc := range cva.OriginalVesting { + for _, ovc := range cva.originalVesting { vestedAmt := sdk.NewDecFromInt(ovc.Amount).Mul(s).RoundInt() vestedCoin := sdk.NewCoin(ovc.Denom, vestedAmt) vestedCoins = vestedCoins.Plus(sdk.Coins{vestedCoin}) @@ -220,7 +219,7 @@ func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coin // GetVestingCoins returns the total number of vesting coins. If no coins are // vesting, nil is returned. func (cva ContinuousVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { - return cva.OriginalVesting.Minus(cva.GetVestedCoins(blockTime)) + return cva.originalVesting.Minus(cva.GetVestedCoins(blockTime)) } // SpendableCoins returns the total number of spendable coins per denom for a @@ -233,22 +232,12 @@ func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coin for _, coin := range bc { baseAmt := coin.Amount - delVestingAmt := cva.DelegatedVesting.AmountOf(coin.Denom) + delVestingAmt := cva.delegatedVesting.AmountOf(coin.Denom) vestingAmt := v.AmountOf(coin.Denom) - a := baseAmt.Add(delVestingAmt) - a = a.Sub(vestingAmt) - - var spendableCoin sdk.Coin - - // compute the min((baseAmt + delVestingAmt) - vestingAmt, baseAmt) - fmt.Println(a) - fmt.Println(baseAmt) - if a.LT(baseAmt) { - spendableCoin = sdk.NewCoin(coin.Denom, a) - } else { - spendableCoin = sdk.NewCoin(coin.Denom, baseAmt) - } + // compute min((BC + DV) - V, BC) per the specification + min := sdk.MinInt(baseAmt.Add(delVestingAmt).Sub(vestingAmt), baseAmt) + spendableCoin := sdk.NewCoin(coin.Denom, min) if !spendableCoin.IsZero() { spendableCoins = spendableCoins.Plus(sdk.Coins{spendableCoin}) @@ -258,6 +247,40 @@ func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coin return spendableCoins } +func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { + bc := cva.GetCoins() + v := cva.GetVestingCoins(blockTime) + + for _, coin := range amount { + // Skip if the delegation amount is zero or if the base coins does not + // exceed the desired delegation amount. + if coin.Amount.IsZero() || bc.AmountOf(coin.Denom).LT(coin.Amount) { + continue + } + + vestingAmt := v.AmountOf(coin.Denom) + delVestingAmt := cva.delegatedVesting.AmountOf(coin.Denom) + + // compute x and y per the specification, where: + // x := min(max(V - DV, 0), D) + // y := D - X + x := sdk.MinInt(sdk.MaxInt(vestingAmt.Sub(delVestingAmt), sdk.ZeroInt()), coin.Amount) + y := coin.Amount.Sub(x) + + if !x.IsZero() { + xCoin := sdk.NewCoin(coin.Denom, x) + cva.delegatedVesting = cva.delegatedVesting.Plus(sdk.Coins{xCoin}) + } + + if !y.IsZero() { + yCoin := sdk.NewCoin(coin.Denom, y) + cva.delegatedFree = cva.delegatedFree.Plus(sdk.Coins{yCoin}) + } + + cva.SetCoins(bc.Minus(sdk.Coins{coin})) + } +} + //----------------------------------------------------------------------------- // Codec diff --git a/x/auth/account_test.go b/x/auth/account_test.go index 096ebd0e1f90..f2816e661c4f 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -192,3 +192,39 @@ func TestSpendableCoinsContVestingAcc(t *testing.T) { spendableCoins = cva.SpendableCoins(now.Add(12 * time.Hour)) require.Nil(t, spendableCoins) } + +func TestTrackDelegationContVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require the ability to delegate all vesting coins + cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(now, origCoins) + require.Equal(t, origCoins, cva.delegatedVesting) + require.Nil(t, cva.delegatedFree) + require.Nil(t, cva.GetCoins()) + + // require the ability to delegate all vested coins + cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(endTime, origCoins) + require.Nil(t, cva.delegatedVesting) + require.Equal(t, origCoins, cva.delegatedFree) + require.Nil(t, cva.GetCoins()) + + // require the ability to delegate all vesting coins (50%) and all vested coins (50%) + cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(now.Add(12*time.Hour), origCoins) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.delegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.delegatedFree) + require.Nil(t, cva.GetCoins()) + + // require no modifications when delegation amount is zero or not enough funds + cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) + require.Nil(t, cva.delegatedVesting) + require.Nil(t, cva.delegatedFree) + require.Equal(t, origCoins, cva.GetCoins()) +} From 5c55471d0133004670fb20d24915a1b4d69a919e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 08:19:17 -0500 Subject: [PATCH 10/76] Further spec revisions --- docs/spec/auth/vesting.md | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index a16f8393d1a8..9cf4ad112aec 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -232,17 +232,19 @@ func (cva ContinuousVestingAccount) TrackDelegation(t Time, amount Coins) { cva.DelegatedVesting += x cva.DelegatedFree += y + cva.SetCoins(cva.GetCoins() - amount) } ``` ##### Delayed/Discrete Vesting Accounts For a delayed vesting account, it can only delegate with received coins and -coins that are fully vested so we only need to update `DF`. +coins that are fully vested so we only need to update `DF` and `BC`. ```go func (dva DelayedVestingAccount) TrackDelegation(t Time, amount Coins) { dva.DelegatedFree += amount + dva.SetCoins(dva.GetCoins() - amount) } ``` @@ -257,7 +259,6 @@ func DelegateCoins(t Time, from Account, amount Coins) { if amount <= sc { from.TrackDelegation(t, amount) - from.SetCoins(sc - amount) // save account... } } else { @@ -279,19 +280,20 @@ For a continuous vesting account attempting to undelegate `D` coins, the following is performed: 1. Verify `(DV + DF) >= D > 0` (this is simply a sanity check) -2. Compute `Y := min(DF, D)` (portion of `D` that should become free, prioritizing free coins) -3. Compute `X := D - Y` (portion of `D` that should remain vesting) -4. Set `DV -= X` -5. Set `DF -= Y` +2. Compute `X := min(DF, D)` (portion of `D` that should become free, prioritizing free coins) +3. Compute `Y := D - X` (portion of `D` that should remain vesting) +4. Set `DF -= X` +5. Set `DV -= Y` 6. Set `BC += D` ```go func (cva ContinuousVestingAccount) TrackUndelegation(amount Coins) { - y := min(cva.DelegatedFree, amount) - x := amount - y + x := min(cva.DelegatedFree, amount) + y := amount - x - cva.DelegatedVesting -= x - cva.DelegatedFree -= y + cva.DelegatedFree -= x + cva.DelegatedVesting -= y + cva.SetCoins(cva.GetCoins() + amount) } ``` @@ -301,12 +303,13 @@ undelegating free coins are prioritized. ##### Delayed/Discrete Vesting Accounts -For a delayed vesting account, it only needs to add back the `DF` amount since -the account is fully vested. +For a delayed vesting account, it only needs to add back the `DF` and `BC` amounts +since the account is fully vested. ```go func (dva DelayedVestingAccount) TrackUndelegation(amount Coins) { dva.DelegatedFree -= amount + dva.SetCoins(dva.GetCoins() + amount) } ``` @@ -317,7 +320,6 @@ func UndelegateCoins(to Account, amount Coins) { if isVesting(to) { if to.DelegatedFree + to.DelegatedVesting >= amount { to.TrackUndelegation(amount) - AddCoins(to, amount) // save account ... } } else { From 7d78e50af2cb92526c712d26200b79ef7b5c5320 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 08:19:40 -0500 Subject: [PATCH 11/76] Add godoc for TrackDelegation and fix comments --- x/auth/account.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 85b7e033b43e..5cb9b1332634 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -247,6 +247,9 @@ func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coin return spendableCoins } +// TrackDelegation tracks a desired delegation amount by setting the appropriate +// values for the amount of delegated vesting, delegated free, and reducing the +// overall amount of base coins. func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { bc := cva.GetCoins() v := cva.GetVestingCoins(blockTime) @@ -262,8 +265,8 @@ func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount delVestingAmt := cva.delegatedVesting.AmountOf(coin.Denom) // compute x and y per the specification, where: - // x := min(max(V - DV, 0), D) - // y := D - X + // X := min(max(V - DV, 0), D) + // Y := D - X x := sdk.MinInt(sdk.MaxInt(vestingAmt.Sub(delVestingAmt), sdk.ZeroInt()), coin.Amount) y := coin.Amount.Sub(x) From 3a0212daa716985e23c12431d273667785ffd763 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 08:59:11 -0500 Subject: [PATCH 12/76] Implement TrackUndelegation for cont. vesting account --- x/auth/account.go | 34 ++++++++++++++++++++++++++++++ x/auth/account_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/x/auth/account.go b/x/auth/account.go index 5cb9b1332634..0cbf1b102180 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -284,6 +284,40 @@ func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount } } +// TrackUndelegation tracks an undelegation amount by setting the necessary +// values by which delegated vesting and delegated vesting need to decrease and +// by which amount the base coins need to increase. +func (cva *ContinuousVestingAccount) TrackUndelegation(amount sdk.Coins) { + bc := cva.GetCoins() + + for _, coin := range amount { + // skip if the undelegation amount is zero + if coin.Amount.IsZero() { + continue + } + + delegatedFree := cva.delegatedFree.AmountOf(coin.Denom) + + // compute x and y per the specification, where: + // X := min(DF, D) + // Y := D - X + x := sdk.MinInt(delegatedFree, coin.Amount) + y := coin.Amount.Sub(x) + + if !x.IsZero() { + xCoin := sdk.NewCoin(coin.Denom, x) + cva.delegatedFree = cva.delegatedFree.Minus(sdk.Coins{xCoin}) + } + + if !y.IsZero() { + yCoin := sdk.NewCoin(coin.Denom, y) + cva.delegatedVesting = cva.delegatedVesting.Minus(sdk.Coins{yCoin}) + } + + cva.SetCoins(bc.Plus(sdk.Coins{coin})) + } +} + //----------------------------------------------------------------------------- // Codec diff --git a/x/auth/account_test.go b/x/auth/account_test.go index f2816e661c4f..4c6fc1b27ecf 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -228,3 +228,51 @@ func TestTrackDelegationContVestingAcc(t *testing.T) { require.Nil(t, cva.delegatedFree) require.Equal(t, origCoins, cva.GetCoins()) } + +func TestTrackUndelegationContVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require the ability to undelegate all vesting coins + cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(now, origCoins) + cva.TrackUndelegation(origCoins) + require.Nil(t, cva.delegatedFree) + require.Nil(t, cva.delegatedVesting) + require.Equal(t, origCoins, cva.GetCoins()) + + // require the ability to undelegate all vested coins + cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(endTime, origCoins) + cva.TrackUndelegation(origCoins) + require.Nil(t, cva.delegatedFree) + require.Nil(t, cva.delegatedVesting) + require.Equal(t, origCoins, cva.GetCoins()) + + // require no modifications when the undelegation amount is zero + cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) + require.Nil(t, cva.delegatedFree) + require.Nil(t, cva.delegatedVesting) + require.Equal(t, origCoins, cva.GetCoins()) + + // vest 50% and delegate to two validators + cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) + cva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + cva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + + // undelegate from one validator that got slashed 50% + cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.delegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.delegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.GetCoins()) + + // undelegate from the other validator that did not get slashed + cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + require.Nil(t, cva.delegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.delegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, cva.GetCoins()) +} From 950e78c0f79a5459c9e2e2edf90ee84032e28a7c Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 09:01:44 -0500 Subject: [PATCH 13/76] Spec interface update --- docs/spec/auth/vesting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 9cf4ad112aec..790f82105ba5 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -59,7 +59,7 @@ type VestingAccount interface { // Calculates the amount of coins that can be sent to other accounts given // the current time. - SpendableCoins(Context) Coins + SpendableCoins(Time) Coins // Performs delegation accounting. TrackDelegation(Time, Coins) // Performs undelegation accounting. From c33758f892b294b8f79851448ede9c7f2ea39d1e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 09:02:07 -0500 Subject: [PATCH 14/76] Assert cont. vesting account interface --- x/auth/account.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 0cbf1b102180..8cd0410c8312 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -38,7 +38,7 @@ type VestingAccount interface { // Calculates the amount of coins that can be sent to other accounts given // the current time. - SpendableCoins(ctx sdk.Context) sdk.Coins + SpendableCoins(blockTime time.Time) sdk.Coins TrackDelegation(blockTime time.Time, amount sdk.Coins) // Performs delegation accounting. TrackUndelegation(amount sdk.Coins) // Performs undelegation accounting. @@ -137,11 +137,11 @@ func (acc *BaseAccount) SetSequence(seq int64) error { //----------------------------------------------------------------------------- // Vesting Accounts -// TODO: uncomment once implemented -// var ( -// _ VestingAccount = (*ContinuousVestingAccount)(nil) -// _ VestingAccount = (*DelayedVestingAccount)(nil) -// ) +var ( + _ VestingAccount = (*ContinuousVestingAccount)(nil) + // TODO: uncomment once implemented + // _ VestingAccount = (*DelayedVestingAccount)(nil) +) type ( // BaseVestingAccount implements the VestingAccount interface. It contains all From 5b7937aa0b649d5f1c2aa3f2388075f46434fffd Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 09:06:25 -0500 Subject: [PATCH 15/76] Fix linting errors --- x/auth/account.go | 10 ++++++---- x/auth/account_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 8cd0410c8312..9a13675e2cac 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -175,8 +175,10 @@ func NewContinuousVestingAccount( addr sdk.AccAddress, origCoins sdk.Coins, startTime, endTime time.Time, ) *ContinuousVestingAccount { - baseAcc := NewBaseAccountWithAddress(addr) - baseAcc.SetCoins(origCoins) + baseAcc := BaseAccount{ + Address: addr, + Coins: origCoins, + } baseVestingAcc := BaseVestingAccount{ BaseAccount: baseAcc, @@ -280,7 +282,7 @@ func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount cva.delegatedFree = cva.delegatedFree.Plus(sdk.Coins{yCoin}) } - cva.SetCoins(bc.Minus(sdk.Coins{coin})) + cva.Coins = bc.Minus(sdk.Coins{coin}) } } @@ -314,7 +316,7 @@ func (cva *ContinuousVestingAccount) TrackUndelegation(amount sdk.Coins) { cva.delegatedVesting = cva.delegatedVesting.Minus(sdk.Coins{yCoin}) } - cva.SetCoins(bc.Plus(sdk.Coins{coin})) + cva.Coins = bc.Plus(sdk.Coins{coin}) } } diff --git a/x/auth/account_test.go b/x/auth/account_test.go index 4c6fc1b27ecf..0e60f421433f 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -121,7 +121,7 @@ func TestGetVestedCoinsContVestingAcc(t *testing.T) { origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) - // require no coins vested in the very begining of the vesting schedule + // require no coins vested in the very beginning of the vesting schedule vestedCoins := cva.GetVestedCoins(now) require.Nil(t, vestedCoins) @@ -142,7 +142,7 @@ func TestGetVestingCoinsContVestingAcc(t *testing.T) { origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) - // require all coins vesting in the begining of the vesting schedule + // require all coins vesting in the beginning of the vesting schedule vestingCoins := cva.GetVestingCoins(now) require.Equal(t, origCoins, vestingCoins) From 9493040ce2836e47d0adb58860f968ff507f5103 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 09:19:23 -0500 Subject: [PATCH 16/76] Implement del. vesting account constructor and spendable coins --- x/auth/account.go | 32 ++++++++++++++++++++++++++++++++ x/auth/account_test.go | 18 ++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/x/auth/account.go b/x/auth/account.go index 9a13675e2cac..33aff0365abf 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -320,6 +320,38 @@ func (cva *ContinuousVestingAccount) TrackUndelegation(amount sdk.Coins) { } } +func NewDelayedVestingAccount( + addr sdk.AccAddress, origCoins sdk.Coins, endTime time.Time, +) *DelayedVestingAccount { + + baseAcc := BaseAccount{ + Address: addr, + Coins: origCoins, + } + + baseVestingAcc := BaseVestingAccount{ + BaseAccount: baseAcc, + originalVesting: origCoins, + endTime: endTime, + } + + return &DelayedVestingAccount{baseVestingAcc} +} + +// GetVestedCoins returns the total amount of vested coins for a delayed vesting +// account. All coins are only vested once the schedule has elapsed. +func (dva DelayedVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { + if blockTime.Unix() >= dva.endTime.Unix() { + return dva.originalVesting + } + + return nil +} + +// func (dva DelayedVestingAccount) GetVestingCoins(t Time) Coins { +// return cva.OriginalVesting - cva.GetVestedCoins(t) +// } + //----------------------------------------------------------------------------- // Codec diff --git a/x/auth/account_test.go b/x/auth/account_test.go index 0e60f421433f..6c621656cb5e 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -276,3 +276,21 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.delegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, cva.GetCoins()) } + +func TestGetVestedCoinsDelVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require no coins are vested until schedule maturation + dva := NewDelayedVestingAccount(addr, origCoins, endTime) + vestedCoins := dva.GetVestedCoins(now) + require.Nil(t, vestedCoins) + + // require all coins be vested at schedule maturation + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + vestedCoins = dva.GetVestedCoins(endTime) + require.Equal(t, origCoins, vestedCoins) +} From be4aaeb8d1cf72dd092d8621b38653ae8f175169 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 10:33:45 -0500 Subject: [PATCH 17/76] Implement GetVestingCoins for del. vesting account --- x/auth/account.go | 8 +++++--- x/auth/account_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 33aff0365abf..c63cf4f358c0 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -348,9 +348,11 @@ func (dva DelayedVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { return nil } -// func (dva DelayedVestingAccount) GetVestingCoins(t Time) Coins { -// return cva.OriginalVesting - cva.GetVestedCoins(t) -// } +// GetVestingCoins returns the total number of vesting coins for a delayed +// vesting account. +func (dva DelayedVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { + return dva.originalVesting.Minus(dva.GetVestedCoins(blockTime)) +} //----------------------------------------------------------------------------- // Codec diff --git a/x/auth/account_test.go b/x/auth/account_test.go index 6c621656cb5e..7295cb620614 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -294,3 +294,21 @@ func TestGetVestedCoinsDelVestingAcc(t *testing.T) { vestedCoins = dva.GetVestedCoins(endTime) require.Equal(t, origCoins, vestedCoins) } + +func TestGetVestingCoinsDelVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require all coins vesting at the beginning of the schedule + dva := NewDelayedVestingAccount(addr, origCoins, endTime) + vestingCoins := dva.GetVestingCoins(now) + require.Equal(t, origCoins, vestingCoins) + + // require no coins vesting at schedule maturation + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + vestingCoins = dva.GetVestingCoins(endTime) + require.Nil(t, vestingCoins) +} From 5fb0d1dcd7c25f25db15b742d99b335b8c879dab Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 11:23:00 -0500 Subject: [PATCH 18/76] Implement SpendableCoins for del. vesting account --- x/auth/account.go | 6 ++++++ x/auth/account_test.go | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index c63cf4f358c0..b3b7ff3e5168 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -354,6 +354,12 @@ func (dva DelayedVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins return dva.originalVesting.Minus(dva.GetVestedCoins(blockTime)) } +// SpendableCoins returns the total number of spendable coins for a delayed +// vesting account. +func (dva DelayedVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { + return dva.GetCoins().Minus(dva.GetVestingCoins(blockTime)) +} + //----------------------------------------------------------------------------- // Codec diff --git a/x/auth/account_test.go b/x/auth/account_test.go index 7295cb620614..a09d3adbd676 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -290,7 +290,6 @@ func TestGetVestedCoinsDelVestingAcc(t *testing.T) { require.Nil(t, vestedCoins) // require all coins be vested at schedule maturation - dva = NewDelayedVestingAccount(addr, origCoins, endTime) vestedCoins = dva.GetVestedCoins(endTime) require.Equal(t, origCoins, vestedCoins) } @@ -308,7 +307,45 @@ func TestGetVestingCoinsDelVestingAcc(t *testing.T) { require.Equal(t, origCoins, vestingCoins) // require no coins vesting at schedule maturation - dva = NewDelayedVestingAccount(addr, origCoins, endTime) vestingCoins = dva.GetVestingCoins(endTime) require.Nil(t, vestingCoins) } + +func TestSpendableCoinsDelVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require that no coins are spendable in the beginning of the vesting + // schedule + dva := NewDelayedVestingAccount(addr, origCoins, endTime) + spendableCoins := dva.SpendableCoins(now) + require.Nil(t, spendableCoins) + + // require that all coins are spendable after the maturation of the vesting + // schedule + spendableCoins = dva.SpendableCoins(endTime) + require.Equal(t, origCoins, spendableCoins) + + // require that all coins are still vesting after some time + spendableCoins = dva.SpendableCoins(now.Add(12 * time.Hour)) + require.Nil(t, spendableCoins) + + // receive some coins + recvAmt := sdk.Coins{sdk.NewInt64Coin(testDenom, 50)} + dva.SetCoins(dva.GetCoins().Plus(recvAmt)) + + // require that only received coins are spendable since the account is still + // vesting + spendableCoins = dva.SpendableCoins(now.Add(12 * time.Hour)) + require.Equal(t, recvAmt, spendableCoins) + + // spend all spendable coins + dva.SetCoins(dva.GetCoins().Minus(spendableCoins)) + + // require that no more coins are spendable + spendableCoins = dva.SpendableCoins(now.Add(12 * time.Hour)) + require.Nil(t, spendableCoins) +} From 5f0229eaef1a584cba74104f8e7d996198fc6125 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 12:42:38 -0500 Subject: [PATCH 19/76] Update spec by simplifying del. vesting account --- docs/spec/auth/vesting.md | 110 ++++++++++++-------------------------- 1 file changed, 34 insertions(+), 76 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 790f82105ba5..128e85236164 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -10,17 +10,11 @@ - [Continuously Vesting Accounts](#continuously-vesting-accounts) - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts) - [Transferring/Sending](#transferringsending) - - [Continuously Vesting Accounts](#continuously-vesting-accounts-1) - - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts-1) - - [Keepers/Handlers](#keepershandlers) + - [Keepers/Handlers](#keepershandlers) - [Delegating](#delegating) - - [Continuously Vesting Accounts](#continuously-vesting-accounts-2) - - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts-2) - - [Keepers/Handlers](#keepershandlers-1) + - [Keepers/Handlers](#keepershandlers-1) - [Undelegating](#undelegating) - - [Continuously Vesting Accounts](#continuously-vesting-accounts-3) - - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts-3) - - [Keepers/Handlers](#keepershandlers-2) + - [Keepers/Handlers](#keepershandlers-2) - [Keepers & Handlers](#keepers--handlers) - [Initializing at Genesis](#initializing-at-genesis) - [Examples](#examples) @@ -145,7 +139,7 @@ func (cva ContinuousVestingAccount) GetVestingCoins(t Time) Coins { #### Delayed/Discrete Vesting Accounts Delayed vesting accounts are easier to reason about as they only have the full -amount vesting up until a certain time, then they all become vested (unlocked). +amount vesting up until a certain time, then all the coins become vested (unlocked). ```go func (dva DelayedVestingAccount) GetVestedCoins(t Time) Coins { @@ -157,40 +151,26 @@ func (dva DelayedVestingAccount) GetVestedCoins(t Time) Coins { } func (dva DelayedVestingAccount) GetVestingCoins(t Time) Coins { - return cva.OriginalVesting - cva.GetVestedCoins(t) + return dva.OriginalVesting - dva.GetVestedCoins(t) } ``` ### Transferring/Sending -#### Continuously Vesting Accounts - -At any given time, a continuous vesting account may transfer: `min((BC + DV) - V, BC)`. +At any given time, a vesting account may transfer: `min((BC + DV) - V, BC)`. In other words, a vesting account may transfer the minimum of the base account balance and the base account balance plus the number of currently delegated vesting coins less the number of coins vested so far. ```go -func (cva ContinuousVestingAccount) SpendableCoins(t Time) Coins { - bc := cva.GetCoins() - return min((bc + cva.DelegatedVesting) - cva.GetVestingCoins(t), bc) -} -``` - -##### Delayed/Discrete Vesting Accounts - -A delayed vesting account may send any coins it has received. In addition, if it -has fully vested, it can send any of it's vested coins. - -```go -func (dva DelayedVestingAccount) SpendableCoins(t Time) Coins { - bc := dva.GetCoins() - return bc - dva.GetVestingCoins(t) +func (va VestingAccount) SpendableCoins(t Time) Coins { + bc := va.GetCoins() + return min((bc + va.DelegatedVesting) - va.GetVestingCoins(t), bc) } ``` -##### Keepers/Handlers +#### Keepers/Handlers The corresponding `x/bank` keeper should appropriately handle sending coins based on if the account is a vesting account or not. @@ -213,10 +193,7 @@ func SendCoins(t Time, from Account, to Account, amount Coins) { ### Delegating -#### Continuously Vesting Accounts - -For a continuous vesting account attempting to delegate `D` coins, the following -is performed: +For a vesting account attempting to delegate `D` coins, the following is performed: 1. Verify `BC >= D > 0` 2. Compute `X := min(max(V - DV, 0), D)` (portion of `D` that is vesting) @@ -226,35 +203,21 @@ is performed: 6. Set `BC -= D` ```go -func (cva ContinuousVestingAccount) TrackDelegation(t Time, amount Coins) { - x := min(max(cva.GetVestingCoins(t) - cva.DelegatedVesting, 0), amount) +func (va VestingAccount) TrackDelegation(t Time, amount Coins) { + x := min(max(va.GetVestingCoins(t) - va.DelegatedVesting, 0), amount) y := amount - x - cva.DelegatedVesting += x - cva.DelegatedFree += y - cva.SetCoins(cva.GetCoins() - amount) -} -``` - -##### Delayed/Discrete Vesting Accounts - -For a delayed vesting account, it can only delegate with received coins and -coins that are fully vested so we only need to update `DF` and `BC`. - -```go -func (dva DelayedVestingAccount) TrackDelegation(t Time, amount Coins) { - dva.DelegatedFree += amount - dva.SetCoins(dva.GetCoins() - amount) + va.DelegatedVesting += x + va.DelegatedFree += y + va.SetCoins(va.GetCoins() - amount) } ``` -##### Keepers/Handlers +#### Keepers/Handlers ```go func DelegateCoins(t Time, from Account, amount Coins) { - // canDelegate checks different semantics for continuous and delayed vesting - // accounts - if isVesting(from) && canDelegate(from) { + if isVesting(from) { sc := from.GetCoins() if amount <= sc { @@ -274,10 +237,7 @@ func DelegateCoins(t Time, from Account, amount Coins) { ### Undelegating -#### Continuously Vesting Accounts - -For a continuous vesting account attempting to undelegate `D` coins, the -following is performed: +For a vesting account attempting to undelegate `D` coins, the following is performed: 1. Verify `(DV + DF) >= D > 0` (this is simply a sanity check) 2. Compute `X := min(DF, D)` (portion of `D` that should become free, prioritizing free coins) @@ -301,19 +261,7 @@ func (cva ContinuousVestingAccount) TrackUndelegation(amount Coins) { with excess an `DV` amount, even after all its coins have vested. This is because undelegating free coins are prioritized. -##### Delayed/Discrete Vesting Accounts - -For a delayed vesting account, it only needs to add back the `DF` and `BC` amounts -since the account is fully vested. - -```go -func (dva DelayedVestingAccount) TrackUndelegation(amount Coins) { - dva.DelegatedFree -= amount - dva.SetCoins(dva.GetCoins() + amount) -} -``` - -##### Keepers/Handlers +#### Keepers/Handlers ```go func UndelegateCoins(to Account, amount Coins) { @@ -352,9 +300,10 @@ BaseAccounts and VestingAccounts as appropriate. ```go type GenesisAccount struct { - Address sdk.AccAddress - GenesisCoins sdk.Coins - EndTime int64 + Address sdk.AccAddress + GenesisCoins sdk.Coins + EndTime int64 + StartTime int64 } func initChainer() { @@ -364,11 +313,20 @@ func initChainer() { Coins: genAcc.GenesisCoins, } - if genAcc.EndTime != 0 { + if genAcc.StartTime != 0 && genAcc.EndTime != 0 { vestingAccount := ContinuousVestingAccount{ BaseAccount: baseAccount, OriginalVesting: genAcc.GenesisCoins, StartTime: RequestInitChain.Time, + StartTime: genAcc.StartTime, + EndTime: genAcc.EndTime, + } + + AddAccountToState(vestingAccount) + } else if genAcc.EndTime != 0 { + vestingAccount := DelayedVestingAccount{ + BaseAccount: baseAccount, + OriginalVesting: genAcc.GenesisCoins, EndTime: genAcc.EndTime, } From 4b5407c9fd9a7e930e58dcd85873f1eb498cae72 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 17:17:42 -0500 Subject: [PATCH 20/76] Cleanup the spec --- docs/spec/auth/vesting.md | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 128e85236164..4f4c56ab7615 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -54,10 +54,12 @@ type VestingAccount interface { // Calculates the amount of coins that can be sent to other accounts given // the current time. SpendableCoins(Time) Coins - // Performs delegation accounting. - TrackDelegation(Time, Coins) - // Performs undelegation accounting. - TrackUndelegation(Coins) + + GetVestedCoins(Time) Coins + GetVestingCoins(Time) Coins + + TrackDelegation(Time, Coins) // Performs delegation accounting. + TrackUndelegation(Coins) // Performs undelegation accounting. } // BaseVestingAccount implements the VestingAccount interface. It contains all @@ -67,7 +69,9 @@ type BaseVestingAccount struct { OriginalVesting Coins // coins in account upon initialization DelegatedFree Coins // coins that are vested and delegated - EndTime Time // when the coins become unlocked + DelegatedVesting Coins // coins that vesting and delegated + + EndTime Time // when the coins become unlocked } // ContinuousVestingAccount implements the VestingAccount interface. It @@ -75,8 +79,7 @@ type BaseVestingAccount struct { type ContinuousVestingAccount struct { BaseVestingAccount - DelegatedVesting Coins // coins that vesting and delegated - StartTime Time // when the coins start to vest + StartTime Time // when the coins start to vest } // DelayedVestingAccount implements the VestingAccount interface. It vests all From fd93ff726e1e466b0e48da48cd1e387979b1f15c Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 17:19:09 -0500 Subject: [PATCH 21/76] DRY-up vesting account implementations --- x/auth/account.go | 241 ++++++++++++++++++++++------------------- x/auth/account_test.go | 37 +++++++ 2 files changed, 167 insertions(+), 111 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index b3b7ff3e5168..b12d1390b0e3 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -40,6 +40,9 @@ type VestingAccount interface { // the current time. SpendableCoins(blockTime time.Time) sdk.Coins + GetVestedCoins(blockTime time.Time) sdk.Coins + GetVestingCoins(blockTime time.Time) sdk.Coins + TrackDelegation(blockTime time.Time, amount sdk.Coins) // Performs delegation accounting. TrackUndelegation(amount sdk.Coins) // Performs undelegation accounting. } @@ -135,107 +138,29 @@ func (acc *BaseAccount) SetSequence(seq int64) error { } //----------------------------------------------------------------------------- -// Vesting Accounts - -var ( - _ VestingAccount = (*ContinuousVestingAccount)(nil) - // TODO: uncomment once implemented - // _ VestingAccount = (*DelayedVestingAccount)(nil) -) - -type ( - // BaseVestingAccount implements the VestingAccount interface. It contains all - // the necessary fields needed for any vesting account implementation. - BaseVestingAccount struct { - BaseAccount - - originalVesting sdk.Coins // coins in account upon initialization - delegatedFree sdk.Coins // coins that are vested and delegated - endTime time.Time // when the coins become unlocked - } - - // ContinuousVestingAccount implements the VestingAccount interface. It - // continuously vests by unlocking coins linearly with respect to time. - ContinuousVestingAccount struct { - BaseVestingAccount - - delegatedVesting sdk.Coins // coins that vesting and delegated - startTime time.Time // when the coins start to vest - } - - // DelayedVestingAccount implements the VestingAccount interface. It vests all - // coins after a specific time, but non prior. In other words, it keeps them - // locked until a specified time. - DelayedVestingAccount struct { - BaseVestingAccount - } -) - -func NewContinuousVestingAccount( - addr sdk.AccAddress, origCoins sdk.Coins, startTime, endTime time.Time, -) *ContinuousVestingAccount { - - baseAcc := BaseAccount{ - Address: addr, - Coins: origCoins, - } - - baseVestingAcc := BaseVestingAccount{ - BaseAccount: baseAcc, - originalVesting: origCoins, - endTime: endTime, - } - - return &ContinuousVestingAccount{ - startTime: startTime, - BaseVestingAccount: baseVestingAcc, - } -} - -// GetVestedCoins returns the total number of vested coins. If no coins are vested, -// nil is returned. -func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { - var vestedCoins sdk.Coins +// Base Vesting Account - // We must handle the case where the start time for a vesting account has - // been set into the future or when the start of the chain is not exactly - // known. - if blockTime.Unix() <= cva.startTime.Unix() { - return vestedCoins - } - - // calculate the vesting scalar - x := blockTime.Unix() - cva.startTime.Unix() - y := cva.endTime.Unix() - cva.startTime.Unix() - s := sdk.NewDec(x).Quo(sdk.NewDec(y)) +// BaseVestingAccount implements the VestingAccount interface. It contains all +// the necessary fields needed for any vesting account implementation. +type BaseVestingAccount struct { + *BaseAccount - for _, ovc := range cva.originalVesting { - vestedAmt := sdk.NewDecFromInt(ovc.Amount).Mul(s).RoundInt() - vestedCoin := sdk.NewCoin(ovc.Denom, vestedAmt) - vestedCoins = vestedCoins.Plus(sdk.Coins{vestedCoin}) - } - - return vestedCoins -} + originalVesting sdk.Coins // coins in account upon initialization + delegatedFree sdk.Coins // coins that are vested and delegated + delegatedVesting sdk.Coins // coins that vesting and delegated -// GetVestingCoins returns the total number of vesting coins. If no coins are -// vesting, nil is returned. -func (cva ContinuousVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { - return cva.originalVesting.Minus(cva.GetVestedCoins(blockTime)) + endTime time.Time // when the coins become unlocked } -// SpendableCoins returns the total number of spendable coins per denom for a -// continuous vesting account. -func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { +func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { var spendableCoins sdk.Coins - bc := cva.GetCoins() - v := cva.GetVestingCoins(blockTime) + bc := bva.GetCoins() for _, coin := range bc { baseAmt := coin.Amount - delVestingAmt := cva.delegatedVesting.AmountOf(coin.Denom) - vestingAmt := v.AmountOf(coin.Denom) + delVestingAmt := bva.delegatedVesting.AmountOf(coin.Denom) + vestingAmt := vestingCoins.AmountOf(coin.Denom) // compute min((BC + DV) - V, BC) per the specification min := sdk.MinInt(baseAmt.Add(delVestingAmt).Sub(vestingAmt), baseAmt) @@ -249,12 +174,8 @@ func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coin return spendableCoins } -// TrackDelegation tracks a desired delegation amount by setting the appropriate -// values for the amount of delegated vesting, delegated free, and reducing the -// overall amount of base coins. -func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { - bc := cva.GetCoins() - v := cva.GetVestingCoins(blockTime) +func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { + bc := bva.GetCoins() for _, coin := range amount { // Skip if the delegation amount is zero or if the base coins does not @@ -263,8 +184,8 @@ func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount continue } - vestingAmt := v.AmountOf(coin.Denom) - delVestingAmt := cva.delegatedVesting.AmountOf(coin.Denom) + vestingAmt := vestingCoins.AmountOf(coin.Denom) + delVestingAmt := bva.delegatedVesting.AmountOf(coin.Denom) // compute x and y per the specification, where: // X := min(max(V - DV, 0), D) @@ -274,23 +195,23 @@ func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - cva.delegatedVesting = cva.delegatedVesting.Plus(sdk.Coins{xCoin}) + bva.delegatedVesting = bva.delegatedVesting.Plus(sdk.Coins{xCoin}) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - cva.delegatedFree = cva.delegatedFree.Plus(sdk.Coins{yCoin}) + bva.delegatedFree = bva.delegatedFree.Plus(sdk.Coins{yCoin}) } - cva.Coins = bc.Minus(sdk.Coins{coin}) + bva.Coins = bc.Minus(sdk.Coins{coin}) } } // TrackUndelegation tracks an undelegation amount by setting the necessary // values by which delegated vesting and delegated vesting need to decrease and // by which amount the base coins need to increase. -func (cva *ContinuousVestingAccount) TrackUndelegation(amount sdk.Coins) { - bc := cva.GetCoins() +func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { + bc := bva.GetCoins() for _, coin := range amount { // skip if the undelegation amount is zero @@ -298,7 +219,7 @@ func (cva *ContinuousVestingAccount) TrackUndelegation(amount sdk.Coins) { continue } - delegatedFree := cva.delegatedFree.AmountOf(coin.Denom) + delegatedFree := bva.delegatedFree.AmountOf(coin.Denom) // compute x and y per the specification, where: // X := min(DF, D) @@ -308,28 +229,119 @@ func (cva *ContinuousVestingAccount) TrackUndelegation(amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - cva.delegatedFree = cva.delegatedFree.Minus(sdk.Coins{xCoin}) + bva.delegatedFree = bva.delegatedFree.Minus(sdk.Coins{xCoin}) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - cva.delegatedVesting = cva.delegatedVesting.Minus(sdk.Coins{yCoin}) + bva.delegatedVesting = bva.delegatedVesting.Minus(sdk.Coins{yCoin}) } - cva.Coins = bc.Plus(sdk.Coins{coin}) + bva.Coins = bc.Plus(sdk.Coins{coin}) + } +} + +//----------------------------------------------------------------------------- +// Continuous Vesting Account + +var _ VestingAccount = (*ContinuousVestingAccount)(nil) + +// ContinuousVestingAccount implements the VestingAccount interface. It +// continuously vests by unlocking coins linearly with respect to time. +type ContinuousVestingAccount struct { + *BaseVestingAccount + + startTime time.Time // when the coins start to vest +} + +func NewContinuousVestingAccount( + addr sdk.AccAddress, origCoins sdk.Coins, startTime, endTime time.Time, +) *ContinuousVestingAccount { + + baseAcc := &BaseAccount{ + Address: addr, + Coins: origCoins, + } + + baseVestingAcc := &BaseVestingAccount{ + BaseAccount: baseAcc, + originalVesting: origCoins, + endTime: endTime, + } + + return &ContinuousVestingAccount{ + startTime: startTime, + BaseVestingAccount: baseVestingAcc, + } +} + +// GetVestedCoins returns the total number of vested coins. If no coins are vested, +// nil is returned. +func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { + var vestedCoins sdk.Coins + + // We must handle the case where the start time for a vesting account has + // been set into the future or when the start of the chain is not exactly + // known. + if blockTime.Unix() <= cva.startTime.Unix() { + return vestedCoins + } + + // calculate the vesting scalar + x := blockTime.Unix() - cva.startTime.Unix() + y := cva.endTime.Unix() - cva.startTime.Unix() + s := sdk.NewDec(x).Quo(sdk.NewDec(y)) + + for _, ovc := range cva.originalVesting { + vestedAmt := sdk.NewDecFromInt(ovc.Amount).Mul(s).RoundInt() + vestedCoin := sdk.NewCoin(ovc.Denom, vestedAmt) + vestedCoins = vestedCoins.Plus(sdk.Coins{vestedCoin}) } + + return vestedCoins +} + +// GetVestingCoins returns the total number of vesting coins. If no coins are +// vesting, nil is returned. +func (cva ContinuousVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { + return cva.originalVesting.Minus(cva.GetVestedCoins(blockTime)) +} + +// SpendableCoins returns the total number of spendable coins per denom for a +// continuous vesting account. +func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { + return cva.spendableCoins(cva.GetVestingCoins(blockTime)) +} + +// TrackDelegation tracks a desired delegation amount by setting the appropriate +// values for the amount of delegated vesting, delegated free, and reducing the +// overall amount of base coins. +func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { + cva.trackDelegation(cva.GetVestingCoins(blockTime), amount) +} + +//----------------------------------------------------------------------------- +// Delayed Vesting Account + +var _ VestingAccount = (*DelayedVestingAccount)(nil) + +// DelayedVestingAccount implements the VestingAccount interface. It vests all +// coins after a specific time, but non prior. In other words, it keeps them +// locked until a specified time. +type DelayedVestingAccount struct { + *BaseVestingAccount } func NewDelayedVestingAccount( addr sdk.AccAddress, origCoins sdk.Coins, endTime time.Time, ) *DelayedVestingAccount { - baseAcc := BaseAccount{ + baseAcc := &BaseAccount{ Address: addr, Coins: origCoins, } - baseVestingAcc := BaseVestingAccount{ + baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, originalVesting: origCoins, endTime: endTime, @@ -357,7 +369,14 @@ func (dva DelayedVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins // SpendableCoins returns the total number of spendable coins for a delayed // vesting account. func (dva DelayedVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { - return dva.GetCoins().Minus(dva.GetVestingCoins(blockTime)) + return dva.spendableCoins(dva.GetVestingCoins(blockTime)) +} + +// TrackDelegation tracks a desired delegation amount by setting the appropriate +// values for the amount of delegated vesting, delegated free, and reducing the +// overall amount of base coins. +func (dva *DelayedVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { + dva.trackDelegation(dva.GetVestingCoins(blockTime), amount) } //----------------------------------------------------------------------------- diff --git a/x/auth/account_test.go b/x/auth/account_test.go index a09d3adbd676..e295c947c32e 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -349,3 +349,40 @@ func TestSpendableCoinsDelVestingAcc(t *testing.T) { spendableCoins = dva.SpendableCoins(now.Add(12 * time.Hour)) require.Nil(t, spendableCoins) } + +func TestTrackDelegationDelVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require the ability to delegate all vesting coins + dva := NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(now, origCoins) + require.Equal(t, origCoins, dva.delegatedVesting) + require.Nil(t, dva.delegatedFree) + require.Nil(t, dva.GetCoins()) + + // require the ability to delegate all vested coins + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(endTime, origCoins) + require.Nil(t, dva.delegatedVesting) + require.Equal(t, origCoins, dva.delegatedFree) + require.Nil(t, dva.GetCoins()) + + // require the ability to delegate all coins half way through the vesting + // schedule + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(now.Add(12*time.Hour), origCoins) + require.Equal(t, origCoins, dva.delegatedVesting) + require.Nil(t, dva.delegatedFree) + require.Nil(t, dva.GetCoins()) + + // require no modifications when delegation amount is zero or not enough funds + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) + require.Nil(t, dva.delegatedVesting) + require.Nil(t, dva.delegatedFree) + require.Equal(t, origCoins, dva.GetCoins()) +} From 73b6b975568fc56ff3d7aba7551e3a524a0e1abc Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 6 Nov 2018 17:35:58 -0500 Subject: [PATCH 22/76] Implement TestTrackUndelegationDelVestingAcc --- x/auth/account_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/x/auth/account_test.go b/x/auth/account_test.go index e295c947c32e..05c7de543789 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -386,3 +386,52 @@ func TestTrackDelegationDelVestingAcc(t *testing.T) { require.Nil(t, dva.delegatedFree) require.Equal(t, origCoins, dva.GetCoins()) } + +func TestTrackUndelegationDelVestingAcc(t *testing.T) { + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + + _, _, addr := keyPubAddr() + origCoins := sdk.Coins{sdk.NewInt64Coin(testDenom, 100)} + + // require the ability to undelegate all vesting coins + dva := NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(now, origCoins) + dva.TrackUndelegation(origCoins) + require.Nil(t, dva.delegatedFree) + require.Nil(t, dva.delegatedVesting) + require.Equal(t, origCoins, dva.GetCoins()) + + // require the ability to undelegate all vested coins + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(endTime, origCoins) + dva.TrackUndelegation(origCoins) + require.Nil(t, dva.delegatedFree) + require.Nil(t, dva.delegatedVesting) + require.Equal(t, origCoins, dva.GetCoins()) + + // require no modifications when the undelegation amount is zero + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) + require.Nil(t, dva.delegatedFree) + require.Nil(t, dva.delegatedVesting) + require.Equal(t, origCoins, dva.GetCoins()) + + // vest 50% and delegate to two validators + dva = NewDelayedVestingAccount(addr, origCoins, endTime) + dva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + dva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + + // undelegate from one validator that got slashed 50% + dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}) + + require.Nil(t, dva.delegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, dva.delegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, dva.GetCoins()) + + // undelegate from the other validator that did not get slashed + dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + require.Nil(t, dva.delegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, dva.delegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, dva.GetCoins()) +} From fc7f71cf2b8e2855a55e38f3fa353d0410ab760a Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 13:32:01 -0500 Subject: [PATCH 23/76] Make all vesting account fields public --- x/auth/account.go | 57 ++++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index b12d1390b0e3..5eceee512df3 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -145,11 +145,11 @@ func (acc *BaseAccount) SetSequence(seq int64) error { type BaseVestingAccount struct { *BaseAccount - originalVesting sdk.Coins // coins in account upon initialization - delegatedFree sdk.Coins // coins that are vested and delegated - delegatedVesting sdk.Coins // coins that vesting and delegated + OriginalVesting sdk.Coins // coins in account upon initialization + DelegatedFree sdk.Coins // coins that are vested and delegated + DelegatedVesting sdk.Coins // coins that vesting and delegated - endTime time.Time // when the coins become unlocked + EndTime time.Time // when the coins become unlocked } func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { @@ -159,7 +159,7 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { for _, coin := range bc { baseAmt := coin.Amount - delVestingAmt := bva.delegatedVesting.AmountOf(coin.Denom) + delVestingAmt := bva.DelegatedVesting.AmountOf(coin.Denom) vestingAmt := vestingCoins.AmountOf(coin.Denom) // compute min((BC + DV) - V, BC) per the specification @@ -185,7 +185,7 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { } vestingAmt := vestingCoins.AmountOf(coin.Denom) - delVestingAmt := bva.delegatedVesting.AmountOf(coin.Denom) + delVestingAmt := bva.DelegatedVesting.AmountOf(coin.Denom) // compute x and y per the specification, where: // X := min(max(V - DV, 0), D) @@ -195,12 +195,12 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - bva.delegatedVesting = bva.delegatedVesting.Plus(sdk.Coins{xCoin}) + bva.DelegatedVesting = bva.DelegatedVesting.Plus(sdk.Coins{xCoin}) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - bva.delegatedFree = bva.delegatedFree.Plus(sdk.Coins{yCoin}) + bva.DelegatedFree = bva.DelegatedFree.Plus(sdk.Coins{yCoin}) } bva.Coins = bc.Minus(sdk.Coins{coin}) @@ -219,22 +219,22 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { continue } - delegatedFree := bva.delegatedFree.AmountOf(coin.Denom) + DelegatedFree := bva.DelegatedFree.AmountOf(coin.Denom) // compute x and y per the specification, where: // X := min(DF, D) // Y := D - X - x := sdk.MinInt(delegatedFree, coin.Amount) + x := sdk.MinInt(DelegatedFree, coin.Amount) y := coin.Amount.Sub(x) if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - bva.delegatedFree = bva.delegatedFree.Minus(sdk.Coins{xCoin}) + bva.DelegatedFree = bva.DelegatedFree.Minus(sdk.Coins{xCoin}) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - bva.delegatedVesting = bva.delegatedVesting.Minus(sdk.Coins{yCoin}) + bva.DelegatedVesting = bva.DelegatedVesting.Minus(sdk.Coins{yCoin}) } bva.Coins = bc.Plus(sdk.Coins{coin}) @@ -251,11 +251,11 @@ var _ VestingAccount = (*ContinuousVestingAccount)(nil) type ContinuousVestingAccount struct { *BaseVestingAccount - startTime time.Time // when the coins start to vest + StartTime time.Time // when the coins start to vest } func NewContinuousVestingAccount( - addr sdk.AccAddress, origCoins sdk.Coins, startTime, endTime time.Time, + addr sdk.AccAddress, origCoins sdk.Coins, StartTime, EndTime time.Time, ) *ContinuousVestingAccount { baseAcc := &BaseAccount{ @@ -265,12 +265,12 @@ func NewContinuousVestingAccount( baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, - originalVesting: origCoins, - endTime: endTime, + OriginalVesting: origCoins, + EndTime: EndTime, } return &ContinuousVestingAccount{ - startTime: startTime, + StartTime: StartTime, BaseVestingAccount: baseVestingAcc, } } @@ -283,16 +283,16 @@ func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coin // We must handle the case where the start time for a vesting account has // been set into the future or when the start of the chain is not exactly // known. - if blockTime.Unix() <= cva.startTime.Unix() { + if blockTime.Unix() <= cva.StartTime.Unix() { return vestedCoins } // calculate the vesting scalar - x := blockTime.Unix() - cva.startTime.Unix() - y := cva.endTime.Unix() - cva.startTime.Unix() + x := blockTime.Unix() - cva.StartTime.Unix() + y := cva.EndTime.Unix() - cva.StartTime.Unix() s := sdk.NewDec(x).Quo(sdk.NewDec(y)) - for _, ovc := range cva.originalVesting { + for _, ovc := range cva.OriginalVesting { vestedAmt := sdk.NewDecFromInt(ovc.Amount).Mul(s).RoundInt() vestedCoin := sdk.NewCoin(ovc.Denom, vestedAmt) vestedCoins = vestedCoins.Plus(sdk.Coins{vestedCoin}) @@ -304,7 +304,7 @@ func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coin // GetVestingCoins returns the total number of vesting coins. If no coins are // vesting, nil is returned. func (cva ContinuousVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { - return cva.originalVesting.Minus(cva.GetVestedCoins(blockTime)) + return cva.OriginalVesting.Minus(cva.GetVestedCoins(blockTime)) } // SpendableCoins returns the total number of spendable coins per denom for a @@ -333,7 +333,7 @@ type DelayedVestingAccount struct { } func NewDelayedVestingAccount( - addr sdk.AccAddress, origCoins sdk.Coins, endTime time.Time, + addr sdk.AccAddress, origCoins sdk.Coins, EndTime time.Time, ) *DelayedVestingAccount { baseAcc := &BaseAccount{ @@ -343,8 +343,8 @@ func NewDelayedVestingAccount( baseVestingAcc := &BaseVestingAccount{ BaseAccount: baseAcc, - originalVesting: origCoins, - endTime: endTime, + OriginalVesting: origCoins, + EndTime: EndTime, } return &DelayedVestingAccount{baseVestingAcc} @@ -353,8 +353,8 @@ func NewDelayedVestingAccount( // GetVestedCoins returns the total amount of vested coins for a delayed vesting // account. All coins are only vested once the schedule has elapsed. func (dva DelayedVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { - if blockTime.Unix() >= dva.endTime.Unix() { - return dva.originalVesting + if blockTime.Unix() >= dva.EndTime.Unix() { + return dva.OriginalVesting } return nil @@ -363,7 +363,7 @@ func (dva DelayedVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coins { // GetVestingCoins returns the total number of vesting coins for a delayed // vesting account. func (dva DelayedVestingAccount) GetVestingCoins(blockTime time.Time) sdk.Coins { - return dva.originalVesting.Minus(dva.GetVestedCoins(blockTime)) + return dva.OriginalVesting.Minus(dva.GetVestedCoins(blockTime)) } // SpendableCoins returns the total number of spendable coins for a delayed @@ -387,6 +387,7 @@ func RegisterBaseAccount(cdc *codec.Codec) { cdc.RegisterInterface((*Account)(nil), nil) cdc.RegisterInterface((*VestingAccount)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/BaseAccount", nil) + cdc.RegisterConcrete(&BaseVestingAccount{}, "cosmos-sdk/BaseVestingAccount", nil) cdc.RegisterConcrete(&ContinuousVestingAccount{}, "cosmos-sdk/ContinuousVestingAccount", nil) cdc.RegisterConcrete(&DelayedVestingAccount{}, "cosmos-sdk/DelayedVestingAccount", nil) codec.RegisterCrypto(cdc) From cf74309f3d6cca955e0d55674c7d1e334fc05a93 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 13:55:55 -0500 Subject: [PATCH 24/76] Update bank keeper SendCoins to handle vesting accounts --- x/bank/keeper.go | 26 ++++++++++++++++++++++---- x/bank/keeper_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 67c05d32068f..74263ca72fdb 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -194,32 +194,50 @@ func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s } // SubtractCoins subtracts amt from the coins at the addr. -func subtractCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { +func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins") - oldCoins := getCoins(ctx, am, addr) + + oldCoins := getCoins(ctx, ak, addr) + + va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) + if ok { + blockTime := ctx.BlockHeader().Time + spendableCoins := va.SpendableCoins(blockTime) + + if !spendableCoins.Minus(amt).IsNotNegative() { + return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) + } + } + newCoins := oldCoins.Minus(amt) if !newCoins.IsNotNegative() { return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) } - err := setCoins(ctx, am, addr, newCoins) + + err := setCoins(ctx, ak, addr, newCoins) tags := sdk.NewTags("sender", []byte(addr.String())) + return newCoins, tags, err } // AddCoins adds amt to the coins at the addr. func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { ctx.GasMeter().ConsumeGas(costAddCoins, "addCoins") + oldCoins := getCoins(ctx, am, addr) newCoins := oldCoins.Plus(amt) + if !newCoins.IsNotNegative() { return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) } + err := setCoins(ctx, am, addr, newCoins) tags := sdk.NewTags("recipient", []byte(addr.String())) + return newCoins, tags, err } -// SendCoins moves coins from one account to another +// SendCoins moves coins from one account to another. // NOTE: Make sure to revert state changes from tx on error func sendCoins(ctx sdk.Context, am auth.AccountKeeper, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { _, subTags, err := subtractCoins(ctx, am, fromAddr, amt) diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go index 26c1446d27c7..53d2ee8731da 100644 --- a/x/bank/keeper_test.go +++ b/x/bank/keeper_test.go @@ -2,6 +2,7 @@ package bank import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -9,6 +10,7 @@ import ( abci "github.com/tendermint/tendermint/abci/types" dbm "github.com/tendermint/tendermint/libs/db" "github.com/tendermint/tendermint/libs/log" + tmtime "github.com/tendermint/tendermint/types/time" codec "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store" @@ -207,3 +209,40 @@ func TestViewKeeper(t *testing.T) { require.False(t, viewKeeper.HasCoins(ctx, addr, sdk.Coins{sdk.NewInt64Coin("foocoin", 15)})) require.False(t, viewKeeper.HasCoins(ctx, addr, sdk.Coins{sdk.NewInt64Coin("barcoin", 5)})) } + +func TestVestingAccountSend(t *testing.T) { + ms, authKey := setupMultiStore() + + cdc := codec.New() + auth.RegisterBaseAccount(cdc) + + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + ctx := sdk.NewContext(ms, abci.Header{Time: now}, false, log.NewNopLogger()) + + origCoins := sdk.Coins{sdk.NewInt64Coin("steak", 100)} + sendCoins := sdk.Coins{sdk.NewInt64Coin("steak", 50)} + + accountKeeper := auth.NewAccountKeeper(cdc, authKey, auth.ProtoBaseAccount) + bankKeeper := NewBaseKeeper(accountKeeper) + + addr1 := sdk.AccAddress([]byte("addr1")) + addr2 := sdk.AccAddress([]byte("addr2")) + vacc := auth.NewContinuousVestingAccount(addr1, origCoins, ctx.BlockHeader().Time, endTime) + accountKeeper.SetAccount(ctx, vacc) + + // require that no coins be sendable at the beginning of the vesting schedule + _, err := bankKeeper.SendCoins(ctx, addr1, addr2, sendCoins) + require.Error(t, err) + + // receive some coins + vacc.SetCoins(origCoins.Plus(sendCoins)) + accountKeeper.SetAccount(ctx, vacc) + + // require that all vested coins are spendable plus any received + ctx = ctx.WithBlockTime(now.Add(12 * time.Hour)) + _, err = bankKeeper.SendCoins(ctx, addr1, addr2, sendCoins) + vacc = accountKeeper.GetAccount(ctx, addr1).(*auth.ContinuousVestingAccount) + require.NoError(t, err) + require.Equal(t, origCoins, vacc.GetCoins()) +} From 4417381f998cc4c2a68d7a7d6f649e6912f9bfa1 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 14:07:13 -0500 Subject: [PATCH 25/76] Implement receive bank keeper test for vesting accounts --- x/bank/keeper_test.go | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go index 53d2ee8731da..07131ef075d6 100644 --- a/x/bank/keeper_test.go +++ b/x/bank/keeper_test.go @@ -212,7 +212,6 @@ func TestViewKeeper(t *testing.T) { func TestVestingAccountSend(t *testing.T) { ms, authKey := setupMultiStore() - cdc := codec.New() auth.RegisterBaseAccount(cdc) @@ -246,3 +245,39 @@ func TestVestingAccountSend(t *testing.T) { require.NoError(t, err) require.Equal(t, origCoins, vacc.GetCoins()) } + +func TestVestingAccountReceive(t *testing.T) { + ms, authKey := setupMultiStore() + cdc := codec.New() + auth.RegisterBaseAccount(cdc) + + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + ctx := sdk.NewContext(ms, abci.Header{Time: now}, false, log.NewNopLogger()) + + origCoins := sdk.Coins{sdk.NewInt64Coin("steak", 100)} + sendCoins := sdk.Coins{sdk.NewInt64Coin("steak", 50)} + + accountKeeper := auth.NewAccountKeeper(cdc, authKey, auth.ProtoBaseAccount) + bankKeeper := NewBaseKeeper(accountKeeper) + + addr1 := sdk.AccAddress([]byte("addr1")) + addr2 := sdk.AccAddress([]byte("addr2")) + + vacc := auth.NewContinuousVestingAccount(addr1, origCoins, ctx.BlockHeader().Time, endTime) + acc := accountKeeper.NewAccountWithAddress(ctx, addr2) + accountKeeper.SetAccount(ctx, vacc) + accountKeeper.SetAccount(ctx, acc) + bankKeeper.SetCoins(ctx, addr2, origCoins) + + // send some coins to the vesting account + bankKeeper.SendCoins(ctx, addr2, addr1, sendCoins) + + // require the coins are spendable + vacc = accountKeeper.GetAccount(ctx, addr1).(*auth.ContinuousVestingAccount) + require.Equal(t, origCoins.Plus(sendCoins), vacc.GetCoins()) + require.Equal(t, vacc.SpendableCoins(now), sendCoins) + + // require coins are spendable plus any that have vested + require.Equal(t, vacc.SpendableCoins(now.Add(12*time.Hour)), origCoins) +} From f241a4b929c77fb4a40b31c91c892047ba7ba96f Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 14:20:51 -0500 Subject: [PATCH 26/76] Cleanup SendCoins pseudo-code in vesting spec --- docs/spec/auth/vesting.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 4f4c56ab7615..c32c292db67e 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -180,17 +180,19 @@ based on if the account is a vesting account or not. ```go func SendCoins(t Time, from Account, to Account, amount Coins) { + bc := from.GetCoins() + if isVesting(from) { sc := from.SpendableCoins(t) - } else { - sc := from.GetCoins() + assert(amount <= sc) } - if amount <= sc { - from.SetCoins(sc - amount) - to.SetCoins(amount) - // save accounts... - } + newCoins := bc - amount + assert(newCoins >= 0) + + from.SetCoins(bc - amount) + to.SetCoins(amount) + // save accounts... } ``` From 97ba3d3e82e5644fc6ed5031bd18c7756cc133d7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 15:08:52 -0500 Subject: [PATCH 27/76] Cleanup DelegateCoins pseudo-code in the vesting spec --- docs/spec/auth/vesting.md | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index c32c292db67e..818208930788 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -222,21 +222,16 @@ func (va VestingAccount) TrackDelegation(t Time, amount Coins) { ```go func DelegateCoins(t Time, from Account, amount Coins) { - if isVesting(from) { - sc := from.GetCoins() + bc := from.GetCoins() + assert(amount <= bc) - if amount <= sc { - from.TrackDelegation(t, amount) - // save account... - } + if isVesting(from) { + from.TrackDelegation(t, amount) } else { - sc := from.GetCoins() - - if amount <= sc { - from.SetCoins(sc - amount) - // save account... - } + from.SetCoins(sc - amount) } + + // save account... } ``` From 5de19657790ebb591028b52871e43329df6c414e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 15:09:31 -0500 Subject: [PATCH 28/76] Add XXX on setting coins directly in track (un)delegation --- x/auth/account.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/auth/account.go b/x/auth/account.go index 5eceee512df3..06eb873bf1f4 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -203,6 +203,7 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { bva.DelegatedFree = bva.DelegatedFree.Plus(sdk.Coins{yCoin}) } + // XXX: this should most likely be handled upstream (i.e. bank keeper) bva.Coins = bc.Minus(sdk.Coins{coin}) } } @@ -237,6 +238,7 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { bva.DelegatedVesting = bva.DelegatedVesting.Minus(sdk.Coins{yCoin}) } + // XXX: this should most likely be handled upstream (i.e. bank keeper) bva.Coins = bc.Plus(sdk.Coins{coin}) } } From a85d199a7f546a8cbc27265e25c93874a01215c5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 7 Nov 2018 15:16:44 -0500 Subject: [PATCH 29/76] Fix value updates in account test --- x/auth/account_test.go | 72 +++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/x/auth/account_test.go b/x/auth/account_test.go index 05c7de543789..2c2a3aa2022c 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -203,29 +203,29 @@ func TestTrackDelegationContVestingAcc(t *testing.T) { // require the ability to delegate all vesting coins cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackDelegation(now, origCoins) - require.Equal(t, origCoins, cva.delegatedVesting) - require.Nil(t, cva.delegatedFree) + require.Equal(t, origCoins, cva.DelegatedVesting) + require.Nil(t, cva.DelegatedFree) require.Nil(t, cva.GetCoins()) // require the ability to delegate all vested coins cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackDelegation(endTime, origCoins) - require.Nil(t, cva.delegatedVesting) - require.Equal(t, origCoins, cva.delegatedFree) + require.Nil(t, cva.DelegatedVesting) + require.Equal(t, origCoins, cva.DelegatedFree) require.Nil(t, cva.GetCoins()) // require the ability to delegate all vesting coins (50%) and all vested coins (50%) cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackDelegation(now.Add(12*time.Hour), origCoins) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.delegatedVesting) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.delegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.DelegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.DelegatedFree) require.Nil(t, cva.GetCoins()) // require no modifications when delegation amount is zero or not enough funds cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) - require.Nil(t, cva.delegatedVesting) - require.Nil(t, cva.delegatedFree) + require.Nil(t, cva.DelegatedVesting) + require.Nil(t, cva.DelegatedFree) require.Equal(t, origCoins, cva.GetCoins()) } @@ -240,23 +240,23 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { cva := NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackDelegation(now, origCoins) cva.TrackUndelegation(origCoins) - require.Nil(t, cva.delegatedFree) - require.Nil(t, cva.delegatedVesting) + require.Nil(t, cva.DelegatedFree) + require.Nil(t, cva.DelegatedVesting) require.Equal(t, origCoins, cva.GetCoins()) // require the ability to undelegate all vested coins cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackDelegation(endTime, origCoins) cva.TrackUndelegation(origCoins) - require.Nil(t, cva.delegatedFree) - require.Nil(t, cva.delegatedVesting) + require.Nil(t, cva.DelegatedFree) + require.Nil(t, cva.DelegatedVesting) require.Equal(t, origCoins, cva.GetCoins()) // require no modifications when the undelegation amount is zero cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) - require.Nil(t, cva.delegatedFree) - require.Nil(t, cva.delegatedVesting) + require.Nil(t, cva.DelegatedFree) + require.Nil(t, cva.DelegatedVesting) require.Equal(t, origCoins, cva.GetCoins()) // vest 50% and delegate to two validators @@ -266,14 +266,14 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { // undelegate from one validator that got slashed 50% cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.delegatedFree) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.delegatedVesting) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.DelegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.DelegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.GetCoins()) // undelegate from the other validator that did not get slashed cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) - require.Nil(t, cva.delegatedFree) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.delegatedVesting) + require.Nil(t, cva.DelegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, cva.DelegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, cva.GetCoins()) } @@ -360,30 +360,30 @@ func TestTrackDelegationDelVestingAcc(t *testing.T) { // require the ability to delegate all vesting coins dva := NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackDelegation(now, origCoins) - require.Equal(t, origCoins, dva.delegatedVesting) - require.Nil(t, dva.delegatedFree) + require.Equal(t, origCoins, dva.DelegatedVesting) + require.Nil(t, dva.DelegatedFree) require.Nil(t, dva.GetCoins()) // require the ability to delegate all vested coins dva = NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackDelegation(endTime, origCoins) - require.Nil(t, dva.delegatedVesting) - require.Equal(t, origCoins, dva.delegatedFree) + require.Nil(t, dva.DelegatedVesting) + require.Equal(t, origCoins, dva.DelegatedFree) require.Nil(t, dva.GetCoins()) // require the ability to delegate all coins half way through the vesting // schedule dva = NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackDelegation(now.Add(12*time.Hour), origCoins) - require.Equal(t, origCoins, dva.delegatedVesting) - require.Nil(t, dva.delegatedFree) + require.Equal(t, origCoins, dva.DelegatedVesting) + require.Nil(t, dva.DelegatedFree) require.Nil(t, dva.GetCoins()) // require no modifications when delegation amount is zero or not enough funds dva = NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) - require.Nil(t, dva.delegatedVesting) - require.Nil(t, dva.delegatedFree) + require.Nil(t, dva.DelegatedVesting) + require.Nil(t, dva.DelegatedFree) require.Equal(t, origCoins, dva.GetCoins()) } @@ -398,23 +398,23 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { dva := NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackDelegation(now, origCoins) dva.TrackUndelegation(origCoins) - require.Nil(t, dva.delegatedFree) - require.Nil(t, dva.delegatedVesting) + require.Nil(t, dva.DelegatedFree) + require.Nil(t, dva.DelegatedVesting) require.Equal(t, origCoins, dva.GetCoins()) // require the ability to undelegate all vested coins dva = NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackDelegation(endTime, origCoins) dva.TrackUndelegation(origCoins) - require.Nil(t, dva.delegatedFree) - require.Nil(t, dva.delegatedVesting) + require.Nil(t, dva.DelegatedFree) + require.Nil(t, dva.DelegatedVesting) require.Equal(t, origCoins, dva.GetCoins()) // require no modifications when the undelegation amount is zero dva = NewDelayedVestingAccount(addr, origCoins, endTime) dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) - require.Nil(t, dva.delegatedFree) - require.Nil(t, dva.delegatedVesting) + require.Nil(t, dva.DelegatedFree) + require.Nil(t, dva.DelegatedVesting) require.Equal(t, origCoins, dva.GetCoins()) // vest 50% and delegate to two validators @@ -425,13 +425,13 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { // undelegate from one validator that got slashed 50% dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}) - require.Nil(t, dva.delegatedFree) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, dva.delegatedVesting) + require.Nil(t, dva.DelegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, dva.DelegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, dva.GetCoins()) // undelegate from the other validator that did not get slashed dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) - require.Nil(t, dva.delegatedFree) - require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, dva.delegatedVesting) + require.Nil(t, dva.DelegatedFree) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 25)}, dva.DelegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 75)}, dva.GetCoins()) } From 6bb0ddd43bbb27cb4925a5d29c3e220d72832f0e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 09:33:11 -0500 Subject: [PATCH 30/76] Implement bank keeper's DelegateCoins --- x/bank/keeper.go | 50 +++++++++++++++++++++++++++++++++++++++++-- x/bank/keeper_test.go | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 74263ca72fdb..73949daa91f5 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -19,9 +19,14 @@ const ( // between accounts. type Keeper interface { SendKeeper + SetCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) sdk.Error SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) + + DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) + // UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) + // DeductFees(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) } var _ Keeper = (*BaseKeeper)(nil) @@ -81,6 +86,21 @@ func (keeper BaseKeeper) InputOutputCoins(ctx sdk.Context, inputs []Input, outpu return inputOutputCoins(ctx, keeper.am, inputs, outputs) } +// DelegateCoins performs delegation by deducting amt coins from an account with +// address addr. For vesting accounts, delegations amounts are tracked for both +// vesting and vested coins. +func (keeper BaseKeeper) DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { + return delegateCoins(ctx, keeper.am, addr, amt) +} + +// func (keeper Keeper) UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { + +// } + +// func (keeper BaseKeeper) DeductFees(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { + +// } + //______________________________________________________________________________________________ // SendKeeper defines a module interface that facilitates the transfer of coins @@ -193,7 +213,9 @@ func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s return getCoins(ctx, am, addr).IsAllGTE(amt) } -// SubtractCoins subtracts amt from the coins at the addr. +// subtractCoins subtracts amt coins from an account with the given address addr. +// +// CONTRACT: If the account is a vesting account, the amount has to be spendable. func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins") @@ -205,7 +227,7 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, spendableCoins := va.SpendableCoins(blockTime) if !spendableCoins.Minus(amt).IsNotNegative() { - return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) + return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", spendableCoins, amt)) } } @@ -276,3 +298,27 @@ func inputOutputCoins(ctx sdk.Context, am auth.AccountKeeper, inputs []Input, ou return allTags, nil } + +func delegateCoins( + ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, +) (sdk.Tags, sdk.Error) { + + ctx.GasMeter().ConsumeGas(costSubtractCoins, "delegateCoins") + + oldCoins := getCoins(ctx, ak, addr) + newCoins := oldCoins.Minus(amt) + + if !newCoins.IsNotNegative() { + return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) + } + + va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) + if ok { + blockTime := ctx.BlockHeader().Time + va.TrackDelegation(blockTime, amt) + } else { + setCoins(ctx, ak, addr, newCoins) + } + + return sdk.NewTags("sender", []byte(addr.String())), nil +} diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go index 07131ef075d6..0c2a7219370b 100644 --- a/x/bank/keeper_test.go +++ b/x/bank/keeper_test.go @@ -281,3 +281,42 @@ func TestVestingAccountReceive(t *testing.T) { // require coins are spendable plus any that have vested require.Equal(t, vacc.SpendableCoins(now.Add(12*time.Hour)), origCoins) } + +func TestDelegateCoins(t *testing.T) { + ms, authKey := setupMultiStore() + cdc := codec.New() + auth.RegisterBaseAccount(cdc) + + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + ctx := sdk.NewContext(ms, abci.Header{Time: now}, false, log.NewNopLogger()) + + origCoins := sdk.Coins{sdk.NewInt64Coin("steak", 100)} + delCoins := sdk.Coins{sdk.NewInt64Coin("steak", 50)} + + accountKeeper := auth.NewAccountKeeper(cdc, authKey, auth.ProtoBaseAccount) + bankKeeper := NewBaseKeeper(accountKeeper) + + addr1 := sdk.AccAddress([]byte("addr1")) + addr2 := sdk.AccAddress([]byte("addr2")) + + vacc := auth.NewContinuousVestingAccount(addr1, origCoins, ctx.BlockHeader().Time, endTime) + acc := accountKeeper.NewAccountWithAddress(ctx, addr2) + accountKeeper.SetAccount(ctx, vacc) + accountKeeper.SetAccount(ctx, acc) + bankKeeper.SetCoins(ctx, addr2, origCoins) + + ctx = ctx.WithBlockTime(now.Add(12 * time.Hour)) + + // require the ability for a non-vesting account to delegate + _, err := bankKeeper.DelegateCoins(ctx, addr2, delCoins) + acc = accountKeeper.GetAccount(ctx, addr2) + require.NoError(t, err) + require.Equal(t, delCoins, acc.GetCoins()) + + // require the ability for a vesting account to delegate + _, err = bankKeeper.DelegateCoins(ctx, addr1, delCoins) + vacc = accountKeeper.GetAccount(ctx, addr1).(*auth.ContinuousVestingAccount) + require.NoError(t, err) + require.Equal(t, delCoins, acc.GetCoins()) +} From 06ec3e5d3941649975163888f593226959c9fef8 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 10:16:19 -0500 Subject: [PATCH 31/76] Implement the bank keeper's UndelegateCoins --- x/bank/keeper.go | 35 ++++++++++++++++++++++++++++----- x/bank/keeper_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 73949daa91f5..76d85f70eb4f 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -25,7 +25,7 @@ type Keeper interface { AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) - // UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) + UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) // DeductFees(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) } @@ -86,16 +86,19 @@ func (keeper BaseKeeper) InputOutputCoins(ctx sdk.Context, inputs []Input, outpu return inputOutputCoins(ctx, keeper.am, inputs, outputs) } -// DelegateCoins performs delegation by deducting amt coins from an account with +// DelegateCoins performs delegation by debiting amt coins from an account with // address addr. For vesting accounts, delegations amounts are tracked for both // vesting and vested coins. func (keeper BaseKeeper) DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { return delegateCoins(ctx, keeper.am, addr, amt) } -// func (keeper Keeper) UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { - -// } +// UndelegateCoins performs undelegation by crediting amt coins to an account with +// address addr. For vesting accounts, undelegation amounts are tracked for both +// vesting and vested coins. +func (keeper BaseKeeper) UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { + return undelegateCoins(ctx, keeper.am, addr, amt) +} // func (keeper BaseKeeper) DeductFees(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { @@ -322,3 +325,25 @@ func delegateCoins( return sdk.NewTags("sender", []byte(addr.String())), nil } + +func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, +) (sdk.Tags, sdk.Error) { + + ctx.GasMeter().ConsumeGas(costAddCoins, "undelegateCoins") + + oldCoins := getCoins(ctx, ak, addr) + newCoins := oldCoins.Plus(amt) + + if !newCoins.IsNotNegative() { + return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) + } + + va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) + if ok { + va.TrackUndelegation(amt) + } else { + setCoins(ctx, ak, addr, newCoins) + } + + return sdk.NewTags("recipient", []byte(addr.String())), nil +} diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go index 0c2a7219370b..a09dce765151 100644 --- a/x/bank/keeper_test.go +++ b/x/bank/keeper_test.go @@ -320,3 +320,48 @@ func TestDelegateCoins(t *testing.T) { require.NoError(t, err) require.Equal(t, delCoins, acc.GetCoins()) } + +func TestUndelegateCoins(t *testing.T) { + ms, authKey := setupMultiStore() + cdc := codec.New() + auth.RegisterBaseAccount(cdc) + + now := tmtime.Now() + endTime := now.Add(24 * time.Hour) + ctx := sdk.NewContext(ms, abci.Header{Time: now}, false, log.NewNopLogger()) + + origCoins := sdk.Coins{sdk.NewInt64Coin("steak", 100)} + delCoins := sdk.Coins{sdk.NewInt64Coin("steak", 50)} + + accountKeeper := auth.NewAccountKeeper(cdc, authKey, auth.ProtoBaseAccount) + bankKeeper := NewBaseKeeper(accountKeeper) + + addr1 := sdk.AccAddress([]byte("addr1")) + addr2 := sdk.AccAddress([]byte("addr2")) + + vacc := auth.NewContinuousVestingAccount(addr1, origCoins, ctx.BlockHeader().Time, endTime) + acc := accountKeeper.NewAccountWithAddress(ctx, addr2) + accountKeeper.SetAccount(ctx, vacc) + accountKeeper.SetAccount(ctx, acc) + bankKeeper.SetCoins(ctx, addr2, origCoins) + + ctx = ctx.WithBlockTime(now.Add(12 * time.Hour)) + + // require the ability for a non-vesting account to undelegate + _, err := bankKeeper.DelegateCoins(ctx, addr2, delCoins) + require.NoError(t, err) + + _, err = bankKeeper.UndelegateCoins(ctx, addr2, delCoins) + require.NoError(t, err) + acc = accountKeeper.GetAccount(ctx, addr2) + require.Equal(t, origCoins, acc.GetCoins()) + + // require the ability for a vesting account to delegate + _, err = bankKeeper.DelegateCoins(ctx, addr1, delCoins) + require.NoError(t, err) + + _, err = bankKeeper.UndelegateCoins(ctx, addr1, delCoins) + require.NoError(t, err) + acc = accountKeeper.GetAccount(ctx, addr1) + require.Equal(t, origCoins, acc.GetCoins()) +} From ce7ac38766cf34f7e31bc2a1e8a8f4c9a6ba8f70 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 10:19:59 -0500 Subject: [PATCH 32/76] Fix vesting account persistence bug --- x/bank/keeper.go | 3 +++ x/bank/keeper_test.go | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 76d85f70eb4f..69962f7a60d0 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -318,7 +318,9 @@ func delegateCoins( va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) if ok { blockTime := ctx.BlockHeader().Time + va.TrackDelegation(blockTime, amt) + ak.SetAccount(ctx, va) } else { setCoins(ctx, ak, addr, newCoins) } @@ -341,6 +343,7 @@ func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) if ok { va.TrackUndelegation(amt) + ak.SetAccount(ctx, va) } else { setCoins(ctx, ak, addr, newCoins) } diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go index a09dce765151..83ff0d1c307d 100644 --- a/x/bank/keeper_test.go +++ b/x/bank/keeper_test.go @@ -318,7 +318,7 @@ func TestDelegateCoins(t *testing.T) { _, err = bankKeeper.DelegateCoins(ctx, addr1, delCoins) vacc = accountKeeper.GetAccount(ctx, addr1).(*auth.ContinuousVestingAccount) require.NoError(t, err) - require.Equal(t, delCoins, acc.GetCoins()) + require.Equal(t, delCoins, vacc.GetCoins()) } func TestUndelegateCoins(t *testing.T) { @@ -362,6 +362,6 @@ func TestUndelegateCoins(t *testing.T) { _, err = bankKeeper.UndelegateCoins(ctx, addr1, delCoins) require.NoError(t, err) - acc = accountKeeper.GetAccount(ctx, addr1) - require.Equal(t, origCoins, acc.GetCoins()) + vacc = accountKeeper.GetAccount(ctx, addr1).(*auth.ContinuousVestingAccount) + require.Equal(t, origCoins, vacc.GetCoins()) } From 999c9fe01ecbccdc2473d7f5c207cc31914a5db5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 10:21:11 -0500 Subject: [PATCH 33/76] Check setCoins error --- x/bank/keeper.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 69962f7a60d0..6a1a59f0859f 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -322,7 +322,9 @@ func delegateCoins( va.TrackDelegation(blockTime, amt) ak.SetAccount(ctx, va) } else { - setCoins(ctx, ak, addr, newCoins) + if err := setCoins(ctx, ak, addr, newCoins); err != nil { + return nil, err + } } return sdk.NewTags("sender", []byte(addr.String())), nil @@ -345,7 +347,9 @@ func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress va.TrackUndelegation(amt) ak.SetAccount(ctx, va) } else { - setCoins(ctx, ak, addr, newCoins) + if err := setCoins(ctx, ak, addr, newCoins); err != nil { + return nil, err + } } return sdk.NewTags("recipient", []byte(addr.String())), nil From 82a0230c0381810c8857d92c3b09d6a1f6efe010 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 11:29:51 -0500 Subject: [PATCH 34/76] Remove DeductFees as subtract coins can be directly used --- x/bank/keeper.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 6a1a59f0859f..274876b79d2b 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -26,7 +26,6 @@ type Keeper interface { DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) - // DeductFees(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) } var _ Keeper = (*BaseKeeper)(nil) @@ -86,7 +85,7 @@ func (keeper BaseKeeper) InputOutputCoins(ctx sdk.Context, inputs []Input, outpu return inputOutputCoins(ctx, keeper.am, inputs, outputs) } -// DelegateCoins performs delegation by debiting amt coins from an account with +// DelegateCoins performs delegation by deducting amt coins from an account with // address addr. For vesting accounts, delegations amounts are tracked for both // vesting and vested coins. func (keeper BaseKeeper) DelegateCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { @@ -100,10 +99,6 @@ func (keeper BaseKeeper) UndelegateCoins(ctx sdk.Context, addr sdk.AccAddress, a return undelegateCoins(ctx, keeper.am, addr, amt) } -// func (keeper BaseKeeper) DeductFees(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error) { - -// } - //______________________________________________________________________________________________ // SendKeeper defines a module interface that facilitates the transfer of coins @@ -223,7 +218,13 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins") oldCoins := getCoins(ctx, ak, addr) + newCoins := oldCoins.Minus(amt) + + if !newCoins.IsNotNegative() { + return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) + } + // for vesting accounts, only 'spendable' coins can be spent va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) if ok { blockTime := ctx.BlockHeader().Time @@ -234,11 +235,6 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, } } - newCoins := oldCoins.Minus(amt) - if !newCoins.IsNotNegative() { - return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) - } - err := setCoins(ctx, ak, addr, newCoins) tags := sdk.NewTags("sender", []byte(addr.String())) From 49cf30bd10f988612cd22cbeeef9b3579b24bdc7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 11:43:42 -0500 Subject: [PATCH 35/76] Update delegation to use new bank keeper API --- x/stake/keeper/delegation.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index fbb62dcbff9a..50f9bebf9621 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -374,10 +374,9 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Co } if subtractAccount { - // Account new shares, save - _, _, err = k.bankKeeper.SubtractCoins(ctx, delegation.DelegatorAddr, sdk.Coins{bondAmt}) + _, err := k.bankKeeper.DelegateCoins(ctx, delegation.DelegatorAddr, sdk.Coins{bondAmt}) if err != nil { - return + return newShares, err } } @@ -506,10 +505,11 @@ func (k Keeper) BeginUnbonding(ctx sdk.Context, // no need to create the ubd object just complete now if completeNow { - _, _, err := k.bankKeeper.AddCoins(ctx, delAddr, sdk.Coins{balance}) + _, err := k.bankKeeper.UndelegateCoins(ctx, delAddr, sdk.Coins{balance}) if err != nil { return types.UnbondingDelegation{}, err } + return types.UnbondingDelegation{MinTime: minTime}, nil } From 50bc71e786229a6b5f2ff302f5a142462a36c877 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 14:59:03 -0500 Subject: [PATCH 36/76] Add pending log entry --- PENDING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PENDING.md b/PENDING.md index 8b2044ea29ee..7785b5d4f13a 100644 --- a/PENDING.md +++ b/PENDING.md @@ -28,7 +28,8 @@ FEATURES * Gaia * SDK - * (#1336) Mechanism for SDK Users to configure their own Bech32 prefixes instead of using the default cosmos prefixes. + * (#1336) Mechanism for SDK Users to configure their own Bech32 prefixes instead of using the default cosmos prefixes. + * \#2694 Vesting account implementation. * Tendermint From 56137f515b1e0d03f639c2faea33677f5cf7ab44 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 17:16:06 -0500 Subject: [PATCH 37/76] Update ante handler to deduct fees for vesting accs --- x/auth/ante.go | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index e88a20a43499..efe02bd0955b 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/hex" "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/tendermint/tendermint/crypto" "github.com/tendermint/tendermint/crypto/ed25519" @@ -85,13 +86,13 @@ func NewAnteHandler(am AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler { return newCtx, res, true } - // first sig pays the fees + // first signer pays the fees if !stdTx.Fee.Amount.IsZero() { - // signerAccs[0] is the fee payer - signerAccs[0], res = deductFees(signerAccs[0], stdTx.Fee) + signerAccs[0], res = deductFees(ctx, signerAccs[0], stdTx.Fee.Amount) if !res.IsOK() { return newCtx, res, true } + fck.AddCollectedFees(newCtx, stdTx.Fee.Amount) } @@ -259,20 +260,32 @@ func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins { // Deduct the fee from the account. // We could use the CoinKeeper (in addition to the AccountKeeper, // because the CoinKeeper doesn't give us accounts), but it seems easier to do this. -func deductFees(acc Account, fee StdFee) (Account, sdk.Result) { - coins := acc.GetCoins() - feeAmount := fee.Amount +func deductFees(ctx sdk.Context, acc Account, fee sdk.Coins) (Account, sdk.Result) { + // TODO: Do we need charge gas for fee deduction + + oldCoins := acc.GetCoins() + newCoins := oldCoins.Minus(fee) - newCoins := coins.Minus(feeAmount) if !newCoins.IsNotNegative() { - errMsg := fmt.Sprintf("%s < %s", coins, feeAmount) - return nil, sdk.ErrInsufficientFunds(errMsg).Result() + return nil, sdk.ErrInsufficientFunds(fmt.Sprintf("%s < %s", oldCoins, fee)).Result() } - err := acc.SetCoins(newCoins) - if err != nil { + + // for vesting accounts, only 'spendable' coins can be used to pay fees + va, ok := acc.(VestingAccount) + if ok { + blockTime := ctx.BlockHeader().Time + spendableCoins := va.SpendableCoins(blockTime) + + if !spendableCoins.Minus(fee).IsNotNegative() { + return nil, sdk.ErrInsufficientFunds(fmt.Sprintf("%s < %s", spendableCoins, fee)).Result() + } + } + + if err := acc.SetCoins(newCoins); err != nil { // Handle w/ #870 panic(err) } + return acc, sdk.Result{} } From d005b3cfe29fdf70a00949aa1984d7c8e1d6957d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 8 Nov 2018 17:17:44 -0500 Subject: [PATCH 38/76] Minor godoc update for deductFees --- x/auth/ante.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index efe02bd0955b..8afcba11e897 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -257,12 +257,10 @@ func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins { return fees.Plus(gasFees) } -// Deduct the fee from the account. -// We could use the CoinKeeper (in addition to the AccountKeeper, -// because the CoinKeeper doesn't give us accounts), but it seems easier to do this. +// deductFees attempts to deduct fees from a given address. Upon success the +// updated account and a result is returned. func deductFees(ctx sdk.Context, acc Account, fee sdk.Coins) (Account, sdk.Result) { - // TODO: Do we need charge gas for fee deduction - + // TODO: Do we need charge gas for fee deduction? oldCoins := acc.GetCoins() newCoins := oldCoins.Minus(fee) From 1ac53f8ad386316afac8542650e8d6b48bc1f769 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 26 Nov 2018 14:00:12 -0500 Subject: [PATCH 39/76] Update delegate/undelegate tags --- x/bank/keeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index a7c8fc089eeb..c95b34e83d1e 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -312,7 +312,7 @@ func delegateCoins( } } - return sdk.NewTags("sender", []byte(addr.String())), nil + return sdk.NewTags("delegateCoins", []byte(addr.String())), nil } func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, @@ -337,5 +337,5 @@ func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress } } - return sdk.NewTags("recipient", []byte(addr.String())), nil + return sdk.NewTags("undelegateCoins", []byte(addr.String())), nil } From ada577f5b14fdb1ef4c55456717e023605bbc82c Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 26 Nov 2018 14:04:27 -0500 Subject: [PATCH 40/76] Add clarification to discrete vesting accounts in spec --- docs/spec/auth/vesting.md | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 818208930788..424d6b4c40c8 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -1,28 +1,6 @@ # Vesting - - -- [Vesting](#vesting) - - [Intro and Requirements](#intro-and-requirements) - - [Vesting Account Types](#vesting-account-types) - - [Vesting Account Specification](#vesting-account-specification) - - [Determining Vesting & Vested Amounts](#determining-vesting--vested-amounts) - - [Continuously Vesting Accounts](#continuously-vesting-accounts) - - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts) - - [Transferring/Sending](#transferringsending) - - [Keepers/Handlers](#keepershandlers) - - [Delegating](#delegating) - - [Keepers/Handlers](#keepershandlers-1) - - [Undelegating](#undelegating) - - [Keepers/Handlers](#keepershandlers-2) - - [Keepers & Handlers](#keepers--handlers) - - [Initializing at Genesis](#initializing-at-genesis) - - [Examples](#examples) - - [Simple](#simple) - - [Slashing](#slashing) - - [Glossary](#glossary) - - +autoauto- [Vesting](#vesting)auto - [Intro and Requirements](#intro-and-requirements)auto - [Vesting Account Types](#vesting-account-types)auto - [Vesting Account Specification](#vesting-account-specification)auto - [Determining Vesting & Vested Amounts](#determining-vesting--vested-amounts)auto - [Continuously Vesting Accounts](#continuously-vesting-accounts)auto - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts)auto - [Transferring/Sending](#transferringsending)auto - [Keepers/Handlers](#keepershandlers)auto - [Delegating](#delegating)auto - [Keepers/Handlers](#keepershandlers-1)auto - [Undelegating](#undelegating)auto - [Keepers/Handlers](#keepershandlers-2)auto - [Keepers & Handlers](#keepers--handlers)auto - [Initializing at Genesis](#initializing-at-genesis)auto - [Examples](#examples)auto - [Simple](#simple)auto - [Slashing](#slashing)auto - [Glossary](#glossary)autoauto ## Intro and Requirements @@ -143,6 +121,7 @@ func (cva ContinuousVestingAccount) GetVestingCoins(t Time) Coins { Delayed vesting accounts are easier to reason about as they only have the full amount vesting up until a certain time, then all the coins become vested (unlocked). +This does not include any unlocked coins the account may have initially. ```go func (dva DelayedVestingAccount) GetVestedCoins(t Time) Coins { From 47d804a32b56a3a6ae21534ccd2d5e675520b9e0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 26 Nov 2018 15:42:11 -0500 Subject: [PATCH 41/76] Add defensive checks to delegation/undelegation amounts --- x/auth/account.go | 4 ++-- x/auth/account_test.go | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 94cce3979484..e2781a066495 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -181,7 +181,7 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { // Skip if the delegation amount is zero or if the base coins does not // exceed the desired delegation amount. if coin.Amount.IsZero() || bc.AmountOf(coin.Denom).LT(coin.Amount) { - continue + panic("delegation attempt with zero coins or insufficient funds") } vestingAmt := vestingCoins.AmountOf(coin.Denom) @@ -217,7 +217,7 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { for _, coin := range amount { // skip if the undelegation amount is zero if coin.Amount.IsZero() { - continue + panic("undelegation attempt with zero coins") } DelegatedFree := bva.DelegatedFree.AmountOf(coin.Denom) diff --git a/x/auth/account_test.go b/x/auth/account_test.go index edcb5eee688d..b0ed01581b7e 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -223,7 +223,10 @@ func TestTrackDelegationContVestingAcc(t *testing.T) { // require no modifications when delegation amount is zero or not enough funds cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) - cva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) + + require.Panics(t, func() { + cva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) + }) require.Nil(t, cva.DelegatedVesting) require.Nil(t, cva.DelegatedFree) require.Equal(t, origCoins, cva.GetCoins()) @@ -254,7 +257,10 @@ func TestTrackUndelegationContVestingAcc(t *testing.T) { // require no modifications when the undelegation amount is zero cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) - cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) + + require.Panics(t, func() { + cva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) + }) require.Nil(t, cva.DelegatedFree) require.Nil(t, cva.DelegatedVesting) require.Equal(t, origCoins, cva.GetCoins()) @@ -381,7 +387,10 @@ func TestTrackDelegationDelVestingAcc(t *testing.T) { // require no modifications when delegation amount is zero or not enough funds dva = NewDelayedVestingAccount(addr, origCoins, endTime) - dva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) + + require.Panics(t, func() { + dva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) + }) require.Nil(t, dva.DelegatedVesting) require.Nil(t, dva.DelegatedFree) require.Equal(t, origCoins, dva.GetCoins()) @@ -412,7 +421,10 @@ func TestTrackUndelegationDelVestingAcc(t *testing.T) { // require no modifications when the undelegation amount is zero dva = NewDelayedVestingAccount(addr, origCoins, endTime) - dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) + + require.Panics(t, func() { + dva.TrackUndelegation(sdk.Coins{sdk.NewInt64Coin(testDenom, 0)}) + }) require.Nil(t, dva.DelegatedFree) require.Nil(t, dva.DelegatedVesting) require.Equal(t, origCoins, dva.GetCoins()) From 03bf9b739a63fde5d72051fb7f755b12bbde9907 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 26 Nov 2018 16:03:35 -0500 Subject: [PATCH 42/76] Update TestTrackDelegationContVestingAcc to test order --- x/auth/account_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x/auth/account_test.go b/x/auth/account_test.go index b0ed01581b7e..b5e9b9a76c0e 100644 --- a/x/auth/account_test.go +++ b/x/auth/account_test.go @@ -216,14 +216,17 @@ func TestTrackDelegationContVestingAcc(t *testing.T) { // require the ability to delegate all vesting coins (50%) and all vested coins (50%) cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) - cva.TrackDelegation(now.Add(12*time.Hour), origCoins) + cva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) + require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.DelegatedVesting) + require.Nil(t, cva.DelegatedFree) + + cva.TrackDelegation(now.Add(12*time.Hour), sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.DelegatedVesting) require.Equal(t, sdk.Coins{sdk.NewInt64Coin(testDenom, 50)}, cva.DelegatedFree) require.Nil(t, cva.GetCoins()) // require no modifications when delegation amount is zero or not enough funds cva = NewContinuousVestingAccount(addr, origCoins, now, endTime) - require.Panics(t, func() { cva.TrackDelegation(endTime, sdk.Coins{sdk.NewInt64Coin(testDenom, 1000000)}) }) From 407e710a2b4cd268c16b3cc77d4bd7dff4143ee6 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 26 Nov 2018 16:21:05 -0500 Subject: [PATCH 43/76] Improve performance of GetVestedCoins --- x/auth/account.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index e2781a066495..775a057eda99 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -290,14 +290,13 @@ func (cva ContinuousVestingAccount) GetVestedCoins(blockTime time.Time) sdk.Coin } // calculate the vesting scalar - x := blockTime.Unix() - cva.StartTime.Unix() - y := cva.EndTime.Unix() - cva.StartTime.Unix() + x := int64(blockTime.Sub(cva.StartTime).Seconds()) + y := int64(cva.EndTime.Sub(cva.StartTime).Seconds()) s := sdk.NewDec(x).Quo(sdk.NewDec(y)) for _, ovc := range cva.OriginalVesting { vestedAmt := sdk.NewDecFromInt(ovc.Amount).Mul(s).RoundInt() - vestedCoin := sdk.NewCoin(ovc.Denom, vestedAmt) - vestedCoins = vestedCoins.Plus(sdk.Coins{vestedCoin}) + vestedCoins = append(vestedCoins, sdk.NewCoin(ovc.Denom, vestedAmt)) } return vestedCoins From eac516c8b54d5c47cbc4f3896ff6d19ff14c5a4b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 09:08:21 -0500 Subject: [PATCH 44/76] Update delegate/undelegate coins tag(s) --- x/bank/keeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index c95b34e83d1e..f68223c7feba 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -312,7 +312,7 @@ func delegateCoins( } } - return sdk.NewTags("delegateCoins", []byte(addr.String())), nil + return sdk.NewTags("action", "delegateCoins"), nil } func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, @@ -337,5 +337,5 @@ func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress } } - return sdk.NewTags("undelegateCoins", []byte(addr.String())), nil + return sdk.NewTags("action", "undelegateCoins"), nil } From ed8cc8041c827164086be57539f943d4a31eb4f2 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 09:57:24 -0500 Subject: [PATCH 45/76] Improve performance of spendableCoins --- x/auth/account.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 775a057eda99..8549dc7ac332 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -152,15 +152,35 @@ type BaseVestingAccount struct { EndTime time.Time // when the coins become unlocked } +// spendableCoins returns all the spendable coins for a vesting account given a +// set of vesting coins. +// +// CONTRACT: vestingCoins and the account's coins must be sorted. func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { var spendableCoins sdk.Coins - bc := bva.GetCoins() + j, k := 0, 0 for _, coin := range bc { + for j < len(vestingCoins) && vestingCoins[j].Denom != coin.Denom { + j++ + } + + for k < len(bva.DelegatedVesting) && bva.DelegatedVesting[k].Denom != coin.Denom { + k++ + } + baseAmt := coin.Amount - delVestingAmt := bva.DelegatedVesting.AmountOf(coin.Denom) - vestingAmt := vestingCoins.AmountOf(coin.Denom) + + vestingAmt := sdk.ZeroInt() + if len(vestingCoins) > 0 { + vestingAmt = vestingCoins[j].Amount + } + + delVestingAmt := sdk.ZeroInt() + if len(bva.DelegatedVesting) > 0 { + delVestingAmt = bva.DelegatedVesting[k].Amount + } // compute min((BC + DV) - V, BC) per the specification min := sdk.MinInt(baseAmt.Add(delVestingAmt).Sub(vestingAmt), baseAmt) From 31c65fa4d02055d3396bc28391e6225d24bf56d5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 09:58:32 -0500 Subject: [PATCH 46/76] Update spendableCoins godoc --- x/auth/account.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/account.go b/x/auth/account.go index 8549dc7ac332..8415ebc02762 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -155,7 +155,8 @@ type BaseVestingAccount struct { // spendableCoins returns all the spendable coins for a vesting account given a // set of vesting coins. // -// CONTRACT: vestingCoins and the account's coins must be sorted. +// CONTRACT: The account's coins, delegated vesting coins, vestingCoins must be +// sorted. func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { var spendableCoins sdk.Coins bc := bva.GetCoins() From 579b12068a0c2b3419a226b6902ab28c11f50a19 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 10:00:15 -0500 Subject: [PATCH 47/76] Further comment/godoc updates --- x/auth/account.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 8415ebc02762..f22ef0167ea9 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -199,7 +199,7 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { bc := bva.GetCoins() for _, coin := range amount { - // Skip if the delegation amount is zero or if the base coins does not + // Panic if the delegation amount is zero or if the base coins does not // exceed the desired delegation amount. if coin.Amount.IsZero() || bc.AmountOf(coin.Denom).LT(coin.Amount) { panic("delegation attempt with zero coins or insufficient funds") @@ -236,7 +236,7 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { bc := bva.GetCoins() for _, coin := range amount { - // skip if the undelegation amount is zero + // panic if the undelegation amount is zero if coin.Amount.IsZero() { panic("undelegation attempt with zero coins") } From d4016ab47c111107147accf29d4980f097c947a6 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 10:21:24 -0500 Subject: [PATCH 48/76] Implement AddCoin and SubCoin --- types/coin.go | 30 +++++++++++++++++++++ types/coin_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/types/coin.go b/types/coin.go index 9742545324be..f0cb39faa2dd 100644 --- a/types/coin.go +++ b/types/coin.go @@ -158,6 +158,18 @@ func (coins Coins) IsValid() bool { } } +// AddCoin adds a single coin to a set of coins. +func (coins Coins) AddCoin(other Coin) Coins { + for i, coin := range coins { + if coin.Denom == other.Denom { + coins[i] = coin.Plus(other) + break + } + } + + return coins +} + // Plus adds two sets of coins. // // e.g. @@ -226,6 +238,24 @@ func (coins Coins) safePlus(coinsB Coins) Coins { } } +// SubCoin subtracts a single coin from a set of coins. +func (coins Coins) SubCoin(other Coin) Coins { + for i, coin := range coins { + if coin.Denom == other.Denom { + otherNeg := Coin{other.Denom, other.Amount.Neg()} + + coins[i] = coin.Plus(otherNeg) + if !coins[i].IsNotNegative() { + panic("negative coin amount") + } + + break + } + } + + return coins +} + // Minus subtracts a set of coins from another. // // e.g. diff --git a/types/coin_test.go b/types/coin_test.go index 774534860d65..910e9acb9201 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -486,3 +486,68 @@ func TestAmountOf(t *testing.T) { assert.Equal(t, NewInt(tc.amountOfTREE), tc.coins.AmountOf("TREE")) } } + +func TestCoinsAddCoin(t *testing.T) { + testCases := []struct { + coins Coins + other Coin + expected Coins + }{ + {Coins{}, Coin{}, Coins{}}, + { + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + NewInt64Coin("A", 5), + Coins{NewInt64Coin("A", 15), NewInt64Coin("B", 5)}, + }, + { + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + NewInt64Coin("C", 5), + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + }, + } + + for i, tc := range testCases { + coins := tc.coins.AddCoin(tc.other) + require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) + } +} + +func TestCoinsSubCoin(t *testing.T) { + testCases := []struct { + coins Coins + other Coin + expected Coins + shouldPanic bool + }{ + {Coins{}, Coin{}, Coins{}, false}, + { + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + NewInt64Coin("A", 5), + Coins{NewInt64Coin("A", 5), NewInt64Coin("B", 5)}, + false, + }, + { + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + NewInt64Coin("C", 5), + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + false, + }, + { + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + NewInt64Coin("A", 15), + Coins{}, + true, + }, + } + + for i, tc := range testCases { + if tc.shouldPanic { + require.Panics(t, func() { + tc.coins.SubCoin(tc.other) + }) + } else { + coins := tc.coins.SubCoin(tc.other) + require.Equal(t, tc.expected, coins, "invalid coins after subtracting a coin; tc #%d", i) + } + } +} From 78dff94c53cf039fbcab4a92acaa182c84a36053 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 11:01:15 -0500 Subject: [PATCH 49/76] Update coins add/sub coin and vesting to use the APIs --- types/coin.go | 28 +++++++++++++++++++++++----- types/coin_test.go | 31 ++++++++++++++++++++++++++----- x/auth/account.go | 8 ++++---- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/types/coin.go b/types/coin.go index f0cb39faa2dd..045838f46d20 100644 --- a/types/coin.go +++ b/types/coin.go @@ -158,8 +158,13 @@ func (coins Coins) IsValid() bool { } } -// AddCoin adds a single coin to a set of coins. -func (coins Coins) AddCoin(other Coin) Coins { +// AddCoinByDenom adds a single coin to another coin in a set of coins by +// denomination. If the coins are empty, the coin is added to the empty set. +func (coins Coins) AddCoinByDenom(other Coin) Coins { + if len(coins) == 0 { + return Coins{other} + } + for i, coin := range coins { if coin.Denom == other.Denom { coins[i] = coin.Plus(other) @@ -238,14 +243,23 @@ func (coins Coins) safePlus(coinsB Coins) Coins { } } -// SubCoin subtracts a single coin from a set of coins. -func (coins Coins) SubCoin(other Coin) Coins { +// SubCoinByDenom subtracts a single coin from another coin in a set of coins by +// denomination. If the coins are empty, the coin is added to the empty set. If +// the resulting coin is zero, it is removed. +func (coins Coins) SubCoinByDenom(other Coin) Coins { + if len(coins) == 0 { + return Coins{other} + } + for i, coin := range coins { if coin.Denom == other.Denom { otherNeg := Coin{other.Denom, other.Amount.Neg()} coins[i] = coin.Plus(otherNeg) - if !coins[i].IsNotNegative() { + if coins[i].IsZero() { + // remove resulting zero coin + coins = append(coins[:i], coins[i+1:]...) + } else if !coins[i].IsNotNegative() { panic("negative coin amount") } @@ -253,6 +267,10 @@ func (coins Coins) SubCoin(other Coin) Coins { } } + if len(coins) == 0 { + return nil + } + return coins } diff --git a/types/coin_test.go b/types/coin_test.go index 910e9acb9201..9596d5aab1bf 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -493,7 +493,6 @@ func TestCoinsAddCoin(t *testing.T) { other Coin expected Coins }{ - {Coins{}, Coin{}, Coins{}}, { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, NewInt64Coin("A", 5), @@ -504,10 +503,15 @@ func TestCoinsAddCoin(t *testing.T) { NewInt64Coin("C", 5), Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, }, + { + Coins{}, + NewInt64Coin("C", 5), + Coins{NewInt64Coin("C", 5)}, + }, } for i, tc := range testCases { - coins := tc.coins.AddCoin(tc.other) + coins := tc.coins.AddCoinByDenom(tc.other) require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) } } @@ -519,7 +523,6 @@ func TestCoinsSubCoin(t *testing.T) { expected Coins shouldPanic bool }{ - {Coins{}, Coin{}, Coins{}, false}, { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, NewInt64Coin("A", 5), @@ -532,6 +535,24 @@ func TestCoinsSubCoin(t *testing.T) { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, false, }, + { + Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + NewInt64Coin("A", 10), + Coins{NewInt64Coin("B", 5)}, + false, + }, + { + Coins{NewInt64Coin("A", 10)}, + NewInt64Coin("A", 10), + nil, + false, + }, + { + Coins{}, + NewInt64Coin("C", 5), + Coins{NewInt64Coin("C", 5)}, + false, + }, { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, NewInt64Coin("A", 15), @@ -543,10 +564,10 @@ func TestCoinsSubCoin(t *testing.T) { for i, tc := range testCases { if tc.shouldPanic { require.Panics(t, func() { - tc.coins.SubCoin(tc.other) + tc.coins.SubCoinByDenom(tc.other) }) } else { - coins := tc.coins.SubCoin(tc.other) + coins := tc.coins.SubCoinByDenom(tc.other) require.Equal(t, tc.expected, coins, "invalid coins after subtracting a coin; tc #%d", i) } } diff --git a/x/auth/account.go b/x/auth/account.go index f22ef0167ea9..7ad717c9d3d7 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -216,12 +216,12 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - bva.DelegatedVesting = bva.DelegatedVesting.Plus(sdk.Coins{xCoin}) + bva.DelegatedVesting = bva.DelegatedVesting.AddCoinByDenom(xCoin) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - bva.DelegatedFree = bva.DelegatedFree.Plus(sdk.Coins{yCoin}) + bva.DelegatedFree = bva.DelegatedFree.AddCoinByDenom(yCoin) } // XXX: this should most likely be handled upstream (i.e. bank keeper) @@ -251,12 +251,12 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - bva.DelegatedFree = bva.DelegatedFree.Minus(sdk.Coins{xCoin}) + bva.DelegatedFree = bva.DelegatedFree.SubCoinByDenom(xCoin) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - bva.DelegatedVesting = bva.DelegatedVesting.Minus(sdk.Coins{yCoin}) + bva.DelegatedVesting = bva.DelegatedVesting.SubCoinByDenom(yCoin) } // XXX: this should most likely be handled upstream (i.e. bank keeper) From ad56f22d0fbe7862bdbf4d8786c5732a9224bebb Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Tue, 27 Nov 2018 11:19:36 -0500 Subject: [PATCH 50/76] Fix tag values (type) --- x/bank/keeper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index f68223c7feba..529a0e460adf 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -312,7 +312,7 @@ func delegateCoins( } } - return sdk.NewTags("action", "delegateCoins"), nil + return sdk.NewTags("action", []byte("delegateCoins")), nil } func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, @@ -337,5 +337,5 @@ func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress } } - return sdk.NewTags("action", "undelegateCoins"), nil + return sdk.NewTags("action", []byte("undelegateCoins")), nil } From 0350362b5ae41862922cda9dcbb2d57dff92e1b8 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 09:05:24 -0500 Subject: [PATCH 51/76] Update Delegate return variable on error --- x/stake/keeper/delegation.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/stake/keeper/delegation.go b/x/stake/keeper/delegation.go index 2140102c6605..971c872a473b 100644 --- a/x/stake/keeper/delegation.go +++ b/x/stake/keeper/delegation.go @@ -406,7 +406,7 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Co if subtractAccount { _, err := k.bankKeeper.DelegateCoins(ctx, delegation.DelegatorAddr, sdk.Coins{bondAmt}) if err != nil { - return newShares, err + return sdk.Dec{}, err } } From 40f5768c25567017e371e4bd6911b15bd7f190f0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 09:10:02 -0500 Subject: [PATCH 52/76] Fix spec TOC --- docs/spec/auth/vesting.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 424d6b4c40c8..37cc89c0b077 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -1,6 +1,24 @@ # Vesting -autoauto- [Vesting](#vesting)auto - [Intro and Requirements](#intro-and-requirements)auto - [Vesting Account Types](#vesting-account-types)auto - [Vesting Account Specification](#vesting-account-specification)auto - [Determining Vesting & Vested Amounts](#determining-vesting--vested-amounts)auto - [Continuously Vesting Accounts](#continuously-vesting-accounts)auto - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts)auto - [Transferring/Sending](#transferringsending)auto - [Keepers/Handlers](#keepershandlers)auto - [Delegating](#delegating)auto - [Keepers/Handlers](#keepershandlers-1)auto - [Undelegating](#undelegating)auto - [Keepers/Handlers](#keepershandlers-2)auto - [Keepers & Handlers](#keepers--handlers)auto - [Initializing at Genesis](#initializing-at-genesis)auto - [Examples](#examples)auto - [Simple](#simple)auto - [Slashing](#slashing)auto - [Glossary](#glossary)autoauto +- [Vesting](#vesting) + - [Intro and Requirements](#intro-and-requirements) + - [Vesting Account Types](#vesting-account-types) + - [Vesting Account Specification](#vesting-account-specification) + - [Determining Vesting & Vested Amounts](#determining-vesting--vested-amounts) + - [Continuously Vesting Accounts](#continuously-vesting-accounts) + - [Delayed/Discrete Vesting Accounts](#delayeddiscrete-vesting-accounts) + - [Transferring/Sending](#transferringsending) + - [Keepers/Handlers](#keepershandlers) + - [Delegating](#delegating) + - [Keepers/Handlers](#keepershandlers-1) + - [Undelegating](#undelegating) + - [Keepers/Handlers](#keepershandlers-2) + - [Keepers & Handlers](#keepers--handlers) + - [Initializing at Genesis](#initializing-at-genesis) + - [Examples](#examples) + - [Simple](#simple) + - [Slashing](#slashing) + - [Glossary](#glossary) ## Intro and Requirements From ef0a9da6878fc35597347b4e6ce4ef39d53e392e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 09:19:31 -0500 Subject: [PATCH 53/76] Update SubCoinByDenom godoc --- types/coin.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/types/coin.go b/types/coin.go index 045838f46d20..ad3a71b130ac 100644 --- a/types/coin.go +++ b/types/coin.go @@ -243,9 +243,10 @@ func (coins Coins) safePlus(coinsB Coins) Coins { } } -// SubCoinByDenom subtracts a single coin from another coin in a set of coins by -// denomination. If the coins are empty, the coin is added to the empty set. If -// the resulting coin is zero, it is removed. +// SubCoinByDenom looks for a matching coin in coins that has the same denom as +// other. Upon matching, the other coin is subtracted from the matching coin in +// place. If the resulting coin is zero, it is removed. If the coins are empty, +// the coin is added to the empty set. func (coins Coins) SubCoinByDenom(other Coin) Coins { if len(coins) == 0 { return Coins{other} From 1fe37b98ed1a14a305b3ad9e3f1cedc868dca53b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 09:21:12 -0500 Subject: [PATCH 54/76] Update AddCoinByDenom godoc --- types/coin.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/types/coin.go b/types/coin.go index ad3a71b130ac..68857b8f81de 100644 --- a/types/coin.go +++ b/types/coin.go @@ -158,8 +158,9 @@ func (coins Coins) IsValid() bool { } } -// AddCoinByDenom adds a single coin to another coin in a set of coins by -// denomination. If the coins are empty, the coin is added to the empty set. +// AddCoinByDenom looks for a matching coin in coins that has the same denom as +// other. Upon matching, the other coin is added to the matching coin in place. +// If the coins are empty, the coin is simply added to the empty set. func (coins Coins) AddCoinByDenom(other Coin) Coins { if len(coins) == 0 { return Coins{other} @@ -246,7 +247,7 @@ func (coins Coins) safePlus(coinsB Coins) Coins { // SubCoinByDenom looks for a matching coin in coins that has the same denom as // other. Upon matching, the other coin is subtracted from the matching coin in // place. If the resulting coin is zero, it is removed. If the coins are empty, -// the coin is added to the empty set. +// the coin is simply added to the empty set. func (coins Coins) SubCoinByDenom(other Coin) Coins { if len(coins) == 0 { return Coins{other} From 129114b1bab1f916ec97c90dc5fb2995640e8ccd Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 09:32:22 -0500 Subject: [PATCH 55/76] Update tags --- x/bank/keeper.go | 14 ++++++++++---- x/bank/tags.go | 10 ++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 x/bank/tags.go diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 529a0e460adf..547080c94b01 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -225,7 +225,7 @@ func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, } err := setCoins(ctx, ak, addr, newCoins) - tags := sdk.NewTags("sender", []byte(addr.String())) + tags := sdk.NewTags(TagKeySender, []byte(addr.String())) return newCoins, tags, err } @@ -242,7 +242,7 @@ func addCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s } err := setCoins(ctx, am, addr, newCoins) - tags := sdk.NewTags("recipient", []byte(addr.String())) + tags := sdk.NewTags(TagKeyRecipient, []byte(addr.String())) return newCoins, tags, err } @@ -312,7 +312,10 @@ func delegateCoins( } } - return sdk.NewTags("action", []byte("delegateCoins")), nil + return sdk.NewTags( + sdk.TagAction, TagActionDelegateCoins, + sdk.TagDelegator, []byte(addr.String()), + ), nil } func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, @@ -337,5 +340,8 @@ func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress } } - return sdk.NewTags("action", []byte("undelegateCoins")), nil + return sdk.NewTags( + sdk.TagAction, TagActionUndelegateCoins, + sdk.TagDelegator, []byte(addr.String()), + ), nil } diff --git a/x/bank/tags.go b/x/bank/tags.go new file mode 100644 index 000000000000..264d8930971d --- /dev/null +++ b/x/bank/tags.go @@ -0,0 +1,10 @@ +package bank + +// Tag keys and values +var ( + TagActionUndelegateCoins = []byte("undelegateCoins") + TagActionDelegateCoins = []byte("delegateCoins") + + TagKeyRecipient = "recipient" + TagKeySender = "sender" +) From 3e4513f912451305ef919fa5c50a95060879ee71 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 09:49:13 -0500 Subject: [PATCH 56/76] Remove XXX comments --- x/auth/account.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 7ad717c9d3d7..909f3fd4e842 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -224,7 +224,6 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { bva.DelegatedFree = bva.DelegatedFree.AddCoinByDenom(yCoin) } - // XXX: this should most likely be handled upstream (i.e. bank keeper) bva.Coins = bc.Minus(sdk.Coins{coin}) } } @@ -259,7 +258,6 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { bva.DelegatedVesting = bva.DelegatedVesting.SubCoinByDenom(yCoin) } - // XXX: this should most likely be handled upstream (i.e. bank keeper) bva.Coins = bc.Plus(sdk.Coins{coin}) } } From cfef5bbc6f2d6e3e2a77ae9f48002354a251e46e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 10:11:42 -0500 Subject: [PATCH 57/76] Improve runtime perf. of track del/undel --- x/auth/account.go | 53 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 909f3fd4e842..6f4cc313d0c3 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -163,10 +163,10 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { j, k := 0, 0 for _, coin := range bc { + // zip/lineup all coins by their denomination to provide O(n) time for j < len(vestingCoins) && vestingCoins[j].Denom != coin.Denom { j++ } - for k < len(bva.DelegatedVesting) && bva.DelegatedVesting[k].Denom != coin.Denom { k++ } @@ -195,19 +195,48 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { return spendableCoins } +// trackDelegation tracks a delegation amount for any given vesting account type +// given the amount of coins currently vesting. +// +// CONTRACT: The account's coins, delegation coins, vesting coins, and delegated +// vesting coins must be sorted. func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { bc := bva.GetCoins() + i, j, k := 0, 0, 0 for _, coin := range amount { + // zip/lineup all coins by their denomination to provide O(n) time + for i < len(bc) && bc[i].Denom != coin.Denom { + i++ + } + for j < len(vestingCoins) && vestingCoins[j].Denom != coin.Denom { + j++ + } + for k < len(bva.DelegatedVesting) && bva.DelegatedVesting[k].Denom != coin.Denom { + k++ + } + + baseAmt := sdk.ZeroInt() + if len(bc) > 0 { + baseAmt = bc[i].Amount + } + + vestingAmt := sdk.ZeroInt() + if len(vestingCoins) > 0 { + vestingAmt = vestingCoins[j].Amount + } + + delVestingAmt := sdk.ZeroInt() + if len(bva.DelegatedVesting) > 0 { + delVestingAmt = bva.DelegatedVesting[k].Amount + } + // Panic if the delegation amount is zero or if the base coins does not // exceed the desired delegation amount. - if coin.Amount.IsZero() || bc.AmountOf(coin.Denom).LT(coin.Amount) { + if coin.Amount.IsZero() || baseAmt.LT(coin.Amount) { panic("delegation attempt with zero coins or insufficient funds") } - vestingAmt := vestingCoins.AmountOf(coin.Denom) - delVestingAmt := bva.DelegatedVesting.AmountOf(coin.Denom) - // compute x and y per the specification, where: // X := min(max(V - DV, 0), D) // Y := D - X @@ -231,21 +260,31 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { // TrackUndelegation tracks an undelegation amount by setting the necessary // values by which delegated vesting and delegated vesting need to decrease and // by which amount the base coins need to increase. +// +// CONTRACT: The account's coins and undelegation coins must be sorted. func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { bc := bva.GetCoins() + i := 0 for _, coin := range amount { // panic if the undelegation amount is zero if coin.Amount.IsZero() { panic("undelegation attempt with zero coins") } - DelegatedFree := bva.DelegatedFree.AmountOf(coin.Denom) + for i < len(bva.DelegatedFree) && bva.DelegatedFree[i].Denom != coin.Denom { + i++ + } + + delegatedFree := sdk.ZeroInt() + if len(bva.DelegatedFree) > 0 { + delegatedFree = bva.DelegatedFree[i].Amount + } // compute x and y per the specification, where: // X := min(DF, D) // Y := D - X - x := sdk.MinInt(DelegatedFree, coin.Amount) + x := sdk.MinInt(delegatedFree, coin.Amount) y := coin.Amount.Sub(x) if !x.IsZero() { From ea4792f74978c7b1dfd27fe5e699f180eb7f465b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 11:13:55 -0500 Subject: [PATCH 58/76] DRY-up subtractCoins --- x/auth/account.go | 15 ++++++++++----- x/bank/keeper.go | 31 ++++++++++++++++++------------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 6f4cc313d0c3..a9e0f98e7643 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -30,16 +30,16 @@ type Account interface { GetCoins() sdk.Coins SetCoins(sdk.Coins) error + + // Calculates the amount of coins that can be sent to other accounts given + // the current time. + SpendableCoins(blockTime time.Time) sdk.Coins } // VestingAccount defines an account type that vests coins via a vesting schedule. type VestingAccount interface { Account - // Calculates the amount of coins that can be sent to other accounts given - // the current time. - SpendableCoins(blockTime time.Time) sdk.Coins - GetVestedCoins(blockTime time.Time) sdk.Coins GetVestingCoins(blockTime time.Time) sdk.Coins @@ -137,6 +137,11 @@ func (acc *BaseAccount) SetSequence(seq uint64) error { return nil } +// SpendableCoins returns the total set of spendable coins. +func (acc *BaseAccount) SpendableCoins(_ time.Time) sdk.Coins { + return acc.GetCoins() +} + //----------------------------------------------------------------------------- // Base Vesting Account @@ -188,7 +193,7 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { spendableCoin := sdk.NewCoin(coin.Denom, min) if !spendableCoin.IsZero() { - spendableCoins = spendableCoins.Plus(sdk.Coins{spendableCoin}) + spendableCoins = spendableCoins.AddCoinByDenom(spendableCoin) } } diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 547080c94b01..54c92320959f 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -13,6 +13,7 @@ const ( costSetCoins sdk.Gas = 100 costSubtractCoins sdk.Gas = 10 costAddCoins sdk.Gas = 10 + costGetAccount sdk.Gas = 10 ) //----------------------------------------------------------------------------- @@ -201,29 +202,33 @@ func hasCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress, amt s return getCoins(ctx, am, addr).IsAllGTE(amt) } +func getAccount(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress) auth.Account { + ctx.GasMeter().ConsumeGas(costGetAccount, "getAccount") + return ak.GetAccount(ctx, addr) +} + // subtractCoins subtracts amt coins from an account with the given address addr. // // CONTRACT: If the account is a vesting account, the amount has to be spendable. func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins") - oldCoins := getCoins(ctx, ak, addr) - newCoins, hasNeg := oldCoins.SafeMinus(amt) - if hasNeg { - return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) - } + oldCoins, spendableCoins := sdk.Coins{}, sdk.Coins{} - // for vesting accounts, only 'spendable' coins can be spent - va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) - if ok { - blockTime := ctx.BlockHeader().Time - spendableCoins := va.SpendableCoins(blockTime) + acc := getAccount(ctx, ak, addr) + if acc != nil { + oldCoins = acc.GetCoins() + spendableCoins = acc.SpendableCoins(ctx.BlockHeader().Time) + } - if _, hasNeg := spendableCoins.SafeMinus(amt); hasNeg { - return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", spendableCoins, amt)) - } + // For non-vesting accounts, spendable coins will simply be the original coins. + // So the check here is sufficient instead of subtracting from oldCoins. + _, hasNeg := spendableCoins.SafeMinus(amt) + if hasNeg { + return amt, nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", spendableCoins, amt)) } + newCoins := oldCoins.Minus(amt) // should not panic as spendable coins was already checked err := setCoins(ctx, ak, addr, newCoins) tags := sdk.NewTags(TagKeySender, []byte(addr.String())) From d1faf4f0ed0815b97e9a278a1edb123e060a1904 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 13:11:36 -0500 Subject: [PATCH 59/76] Update Add/SubCoinByDenom to work on a copy --- types/coin.go | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/types/coin.go b/types/coin.go index 68857b8f81de..6f07ed054623 100644 --- a/types/coin.go +++ b/types/coin.go @@ -159,21 +159,23 @@ func (coins Coins) IsValid() bool { } // AddCoinByDenom looks for a matching coin in coins that has the same denom as -// other. Upon matching, the other coin is added to the matching coin in place. -// If the coins are empty, the coin is simply added to the empty set. +// other. Upon matching, the other coin is added to the matching coin. If the +// coins are empty, the coin is simply added to the empty set. func (coins Coins) AddCoinByDenom(other Coin) Coins { - if len(coins) == 0 { + res := copyCoins(coins) + + if len(res) == 0 { return Coins{other} } - for i, coin := range coins { + for i, coin := range res { if coin.Denom == other.Denom { - coins[i] = coin.Plus(other) + res[i] = coin.Plus(other) break } } - return coins + return res } // Plus adds two sets of coins. @@ -245,23 +247,25 @@ func (coins Coins) safePlus(coinsB Coins) Coins { } // SubCoinByDenom looks for a matching coin in coins that has the same denom as -// other. Upon matching, the other coin is subtracted from the matching coin in -// place. If the resulting coin is zero, it is removed. If the coins are empty, -// the coin is simply added to the empty set. +// other. Upon matching, the other coin is subtracted from the matching coin. If +// the resulting coin is zero, it is removed. If the coins are empty, the coin +// is simply added to the empty set. func (coins Coins) SubCoinByDenom(other Coin) Coins { - if len(coins) == 0 { + res := copyCoins(coins) + + if len(res) == 0 { return Coins{other} } - for i, coin := range coins { + for i, coin := range res { if coin.Denom == other.Denom { otherNeg := Coin{other.Denom, other.Amount.Neg()} - coins[i] = coin.Plus(otherNeg) - if coins[i].IsZero() { + res[i] = coin.Plus(otherNeg) + if res[i].IsZero() { // remove resulting zero coin - coins = append(coins[:i], coins[i+1:]...) - } else if !coins[i].IsNotNegative() { + res = append(res[:i], res[i+1:]...) + } else if !res[i].IsNotNegative() { panic("negative coin amount") } @@ -269,11 +273,11 @@ func (coins Coins) SubCoinByDenom(other Coin) Coins { } } - if len(coins) == 0 { + if len(res) == 0 { return nil } - return coins + return res } // Minus subtracts a set of coins from another. @@ -463,6 +467,12 @@ func removeZeroCoins(coins Coins) Coins { return coins[:i] } +func copyCoins(coins Coins) Coins { + copyCoins := make(Coins, len(coins)) + copy(copyCoins, coins) + return copyCoins +} + //----------------------------------------------------------------------------- // Sort interface From 21fa5b461ce3cf194c8449b338740559f1a219e8 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 13:12:33 -0500 Subject: [PATCH 60/76] Update vesting accounts API --- x/auth/account.go | 51 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index a9e0f98e7643..64290e4c32e5 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -34,6 +34,11 @@ type Account interface { // Calculates the amount of coins that can be sent to other accounts given // the current time. SpendableCoins(blockTime time.Time) sdk.Coins + + // Delegation and undelegation accounting that returns the resulting base + // coins amount. + TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins + TrackUndelegation(amount sdk.Coins) sdk.Coins } // VestingAccount defines an account type that vests coins via a vesting schedule. @@ -42,9 +47,6 @@ type VestingAccount interface { GetVestedCoins(blockTime time.Time) sdk.Coins GetVestingCoins(blockTime time.Time) sdk.Coins - - TrackDelegation(blockTime time.Time, amount sdk.Coins) // Performs delegation accounting. - TrackUndelegation(amount sdk.Coins) // Performs undelegation accounting. } // AccountDecoder unmarshals account bytes @@ -137,11 +139,24 @@ func (acc *BaseAccount) SetSequence(seq uint64) error { return nil } -// SpendableCoins returns the total set of spendable coins. +// SpendableCoins returns the total set of spendable coins. For a base account, +// this is simply the base coins. func (acc *BaseAccount) SpendableCoins(_ time.Time) sdk.Coins { return acc.GetCoins() } +// TrackDelegation performs delegation accounting. For a base account it simply +// returns the base coins minus the desired delegation amount. +func (acc *BaseAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { + return acc.GetCoins().Minus(amount) +} + +// TrackUndelegation performs undelegation accounting. For a base account it +// simply returns the base coins plus the undelegation amount. +func (acc *BaseAccount) TrackUndelegation(amount sdk.Coins) sdk.Coins { + return acc.GetCoins().Plus(amount) +} + //----------------------------------------------------------------------------- // Base Vesting Account @@ -201,11 +216,12 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { } // trackDelegation tracks a delegation amount for any given vesting account type -// given the amount of coins currently vesting. +// given the amount of coins currently vesting. It returns the resulting base +// coins. // // CONTRACT: The account's coins, delegation coins, vesting coins, and delegated // vesting coins must be sorted. -func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { +func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) sdk.Coins { bc := bva.GetCoins() i, j, k := 0, 0, 0 @@ -258,18 +274,19 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { bva.DelegatedFree = bva.DelegatedFree.AddCoinByDenom(yCoin) } - bva.Coins = bc.Minus(sdk.Coins{coin}) + bva.Coins = bva.Coins.SubCoinByDenom(coin) } + + return bva.Coins } // TrackUndelegation tracks an undelegation amount by setting the necessary // values by which delegated vesting and delegated vesting need to decrease and -// by which amount the base coins need to increase. +// by which amount the base coins need to increase. The resulting base coins are +// returned. // // CONTRACT: The account's coins and undelegation coins must be sorted. -func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { - bc := bva.GetCoins() - +func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) sdk.Coins { i := 0 for _, coin := range amount { // panic if the undelegation amount is zero @@ -302,8 +319,10 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { bva.DelegatedVesting = bva.DelegatedVesting.SubCoinByDenom(yCoin) } - bva.Coins = bc.Plus(sdk.Coins{coin}) + bva.Coins = bva.Coins.AddCoinByDenom(coin) } + + return bva.Coins } //----------------------------------------------------------------------------- @@ -380,8 +399,8 @@ func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coin // TrackDelegation tracks a desired delegation amount by setting the appropriate // values for the amount of delegated vesting, delegated free, and reducing the // overall amount of base coins. -func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { - cva.trackDelegation(cva.GetVestingCoins(blockTime), amount) +func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { + return cva.trackDelegation(cva.GetVestingCoins(blockTime), amount) } //----------------------------------------------------------------------------- @@ -439,8 +458,8 @@ func (dva DelayedVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { // TrackDelegation tracks a desired delegation amount by setting the appropriate // values for the amount of delegated vesting, delegated free, and reducing the // overall amount of base coins. -func (dva *DelayedVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { - dva.trackDelegation(dva.GetVestingCoins(blockTime), amount) +func (dva *DelayedVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { + return dva.trackDelegation(dva.GetVestingCoins(blockTime), amount) } //----------------------------------------------------------------------------- From 3d4bee2130a59be5f4862856f0f197024eaae27b Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Nov 2018 13:12:52 -0500 Subject: [PATCH 61/76] DRY-up delegation/undelegation in bank keeper --- x/bank/keeper.go | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 54c92320959f..158279f1df8d 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -212,7 +212,6 @@ func getAccount(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress) aut // CONTRACT: If the account is a vesting account, the amount has to be spendable. func subtractCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error) { ctx.GasMeter().ConsumeGas(costSubtractCoins, "subtractCoins") - oldCoins, spendableCoins := sdk.Coins{}, sdk.Coins{} acc := getAccount(ctx, ak, addr) @@ -298,23 +297,21 @@ func delegateCoins( ctx.GasMeter().ConsumeGas(costSubtractCoins, "delegateCoins") - oldCoins := getCoins(ctx, ak, addr) - newCoins := oldCoins.Minus(amt) + acc := getAccount(ctx, ak, addr) + if acc == nil { + return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) + } + + oldCoins := acc.GetCoins() - if !newCoins.IsNotNegative() { + _, hasNeg := oldCoins.SafeMinus(amt) + if hasNeg { return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) } - va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) - if ok { - blockTime := ctx.BlockHeader().Time - - va.TrackDelegation(blockTime, amt) - ak.SetAccount(ctx, va) - } else { - if err := setCoins(ctx, ak, addr, newCoins); err != nil { - return nil, err - } + newCoins := acc.TrackDelegation(ctx.BlockHeader().Time, amt) + if err := setCoins(ctx, ak, addr, newCoins); err != nil { + return nil, err } return sdk.NewTags( @@ -323,26 +320,20 @@ func delegateCoins( ), nil } -func undelegateCoins(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, +func undelegateCoins( + ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress, amt sdk.Coins, ) (sdk.Tags, sdk.Error) { ctx.GasMeter().ConsumeGas(costAddCoins, "undelegateCoins") - oldCoins := getCoins(ctx, ak, addr) - newCoins := oldCoins.Plus(amt) - - if !newCoins.IsNotNegative() { - return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) + acc := getAccount(ctx, ak, addr) + if acc == nil { + return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) } - va, ok := ak.GetAccount(ctx, addr).(auth.VestingAccount) - if ok { - va.TrackUndelegation(amt) - ak.SetAccount(ctx, va) - } else { - if err := setCoins(ctx, ak, addr, newCoins); err != nil { - return nil, err - } + newCoins := acc.TrackUndelegation(amt) + if err := setCoins(ctx, ak, addr, newCoins); err != nil { + return nil, err } return sdk.NewTags( From 511dc6865cb92aaaf11c700979b52693ae8ede9e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 5 Dec 2018 09:19:32 -0500 Subject: [PATCH 62/76] Update vesting spec accounts section --- docs/spec/auth/vesting.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 37cc89c0b077..b25ae7f60f6b 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -47,15 +47,8 @@ order to make such a distinction. type VestingAccount interface { Account - // Calculates the amount of coins that can be sent to other accounts given - // the current time. - SpendableCoins(Time) Coins - GetVestedCoins(Time) Coins GetVestingCoins(Time) Coins - - TrackDelegation(Time, Coins) // Performs delegation accounting. - TrackUndelegation(Coins) // Performs undelegation accounting. } // BaseVestingAccount implements the VestingAccount interface. It contains all @@ -86,6 +79,25 @@ type DelayedVestingAccount struct { } ``` +In order to facilitate less ad-hoc type checking and assertions and to support +flexibility in account usage, the existing `Account` interface is updated to contain +the following: + +```go +type Account interface { + // ... + + // Calculates the amount of coins that can be sent to other accounts given + // the current time. + SpendableCoins(Time) Coins + + // Delegation and undelegation accounting that returns the resulting base + // coins amount. + TrackDelegation(Time, Coins) + TrackUndelegation(Coins) +} +``` + ## Vesting Account Specification Given a vesting account, we define the following in the proceeding operations: From 653434c09310b4f504f02f851cb2080ffbf10584 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 5 Dec 2018 09:22:38 -0500 Subject: [PATCH 63/76] Cleanup deductFees --- x/auth/ante.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index 226e558ef4bd..a229fb91855c 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/hex" "fmt" + "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/tendermint/tendermint/crypto" @@ -89,7 +90,7 @@ func NewAnteHandler(am AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler { // first signer pays the fees if !stdTx.Fee.Amount.IsZero() { - signerAccs[0], res = deductFees(ctx, signerAccs[0], stdTx.Fee.Amount) + signerAccs[0], res = deductFees(ctx.BlockHeader().Time, signerAccs[0], stdTx.Fee.Amount) if !res.IsOK() { return newCtx, res, true } @@ -220,7 +221,7 @@ func adjustFeesByGas(fees sdk.Coins, gas uint64) sdk.Coins { // deductFees attempts to deduct fees from a given address. Upon success the // updated account and a result is returned. -func deductFees(ctx sdk.Context, acc Account, fee sdk.Coins) (Account, sdk.Result) { +func deductFees(blockTime time.Time, acc Account, fee sdk.Coins) (Account, sdk.Result) { if !fee.IsValid() { return nil, sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee amount: %s", fee)).Result() } @@ -236,7 +237,7 @@ func deductFees(ctx sdk.Context, acc Account, fee sdk.Coins) (Account, sdk.Resul // Validate the account has enough "spendable" coins as this will cover cases // such as vesting accounts. - spendableCoins := acc.SpendableCoins(ctx.BlockHeader().Time) + spendableCoins := acc.SpendableCoins(blockTime) if _, hasNeg := spendableCoins.SafeMinus(fee); hasNeg { return nil, sdk.ErrInsufficientFunds(fmt.Sprintf("%s < %s", spendableCoins, fee)).Result() } From 5b969c7bd220bd4bc51af5cc9e747b4238f1e739 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 5 Dec 2018 12:34:39 -0500 Subject: [PATCH 64/76] Remove Coin#IsNotNegative --- types/coin.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/types/coin.go b/types/coin.go index 6f07ed054623..c1f44348f767 100644 --- a/types/coin.go +++ b/types/coin.go @@ -93,7 +93,7 @@ func (coin Coin) Minus(coinB Coin) Coin { } res := Coin{coin.Denom, coin.Amount.Sub(coinB.Amount)} - if !res.IsNotNegative() { + if res.IsNegative() { panic("negative count amount") } @@ -104,14 +104,14 @@ func (coin Coin) Minus(coinB Coin) Coin { // // TODO: Remove once unsigned integers are used. func (coin Coin) IsPositive() bool { - return (coin.Amount.Sign() == 1) + return coin.Amount.Sign() == 1 } -// IsNotNegative returns true if coin amount is not negative and false otherwise. +// IsNegative returns true if the coin amount is negative and false otherwise. // // TODO: Remove once unsigned integers are used. -func (coin Coin) IsNotNegative() bool { - return (coin.Amount.Sign() != -1) +func (coin Coin) IsNegative() bool { + return coin.Amount.Sign() == -1 } //----------------------------------------------------------------------------- @@ -265,7 +265,7 @@ func (coins Coins) SubCoinByDenom(other Coin) Coins { if res[i].IsZero() { // remove resulting zero coin res = append(res[:i], res[i+1:]...) - } else if !res[i].IsNotNegative() { + } else if res[i].IsNegative() { panic("negative coin amount") } @@ -427,7 +427,7 @@ func (coins Coins) IsNotNegative() bool { } for _, coin := range coins { - if !coin.IsNotNegative() { + if coin.IsNegative() { return false } } From 19991a484393f69a674919f7a30e176783baddfa Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 5 Dec 2018 13:00:56 -0500 Subject: [PATCH 65/76] Modify base account in TrackDelegation/TrackUndelegation --- x/auth/account.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 64290e4c32e5..ba0163a2d1e5 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -148,13 +148,15 @@ func (acc *BaseAccount) SpendableCoins(_ time.Time) sdk.Coins { // TrackDelegation performs delegation accounting. For a base account it simply // returns the base coins minus the desired delegation amount. func (acc *BaseAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { - return acc.GetCoins().Minus(amount) + acc.Coins = acc.Coins.Minus(amount) + return acc.Coins } // TrackUndelegation performs undelegation accounting. For a base account it // simply returns the base coins plus the undelegation amount. func (acc *BaseAccount) TrackUndelegation(amount sdk.Coins) sdk.Coins { - return acc.GetCoins().Plus(amount) + acc.Coins = acc.Coins.Plus(amount) + return acc.Coins } //----------------------------------------------------------------------------- From 01cb81192baa45de2b5bb76d5743d83ea6d77fd0 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 5 Dec 2018 14:28:37 -0500 Subject: [PATCH 66/76] Dont return coins on TrackUndelegation/TrackDelegation --- x/auth/account.go | 30 ++++++++++++------------------ x/bank/keeper.go | 18 ++++++++++-------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index ba0163a2d1e5..5cd1a21f57ab 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -37,8 +37,8 @@ type Account interface { // Delegation and undelegation accounting that returns the resulting base // coins amount. - TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins - TrackUndelegation(amount sdk.Coins) sdk.Coins + TrackDelegation(blockTime time.Time, amount sdk.Coins) + TrackUndelegation(amount sdk.Coins) } // VestingAccount defines an account type that vests coins via a vesting schedule. @@ -146,17 +146,15 @@ func (acc *BaseAccount) SpendableCoins(_ time.Time) sdk.Coins { } // TrackDelegation performs delegation accounting. For a base account it simply -// returns the base coins minus the desired delegation amount. -func (acc *BaseAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { +// sets the base coins minus the desired delegation amount. +func (acc *BaseAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { acc.Coins = acc.Coins.Minus(amount) - return acc.Coins } // TrackUndelegation performs undelegation accounting. For a base account it -// simply returns the base coins plus the undelegation amount. -func (acc *BaseAccount) TrackUndelegation(amount sdk.Coins) sdk.Coins { +// simply sets the base coins plus the undelegation amount. +func (acc *BaseAccount) TrackUndelegation(amount sdk.Coins) { acc.Coins = acc.Coins.Plus(amount) - return acc.Coins } //----------------------------------------------------------------------------- @@ -223,7 +221,7 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { // // CONTRACT: The account's coins, delegation coins, vesting coins, and delegated // vesting coins must be sorted. -func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) sdk.Coins { +func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { bc := bva.GetCoins() i, j, k := 0, 0, 0 @@ -278,8 +276,6 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) s bva.Coins = bva.Coins.SubCoinByDenom(coin) } - - return bva.Coins } // TrackUndelegation tracks an undelegation amount by setting the necessary @@ -288,7 +284,7 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) s // returned. // // CONTRACT: The account's coins and undelegation coins must be sorted. -func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) sdk.Coins { +func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { i := 0 for _, coin := range amount { // panic if the undelegation amount is zero @@ -323,8 +319,6 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) sdk.Coins { bva.Coins = bva.Coins.AddCoinByDenom(coin) } - - return bva.Coins } //----------------------------------------------------------------------------- @@ -401,8 +395,8 @@ func (cva ContinuousVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coin // TrackDelegation tracks a desired delegation amount by setting the appropriate // values for the amount of delegated vesting, delegated free, and reducing the // overall amount of base coins. -func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { - return cva.trackDelegation(cva.GetVestingCoins(blockTime), amount) +func (cva *ContinuousVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { + cva.trackDelegation(cva.GetVestingCoins(blockTime), amount) } //----------------------------------------------------------------------------- @@ -460,8 +454,8 @@ func (dva DelayedVestingAccount) SpendableCoins(blockTime time.Time) sdk.Coins { // TrackDelegation tracks a desired delegation amount by setting the appropriate // values for the amount of delegated vesting, delegated free, and reducing the // overall amount of base coins. -func (dva *DelayedVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) sdk.Coins { - return dva.trackDelegation(dva.GetVestingCoins(blockTime), amount) +func (dva *DelayedVestingAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { + dva.trackDelegation(dva.GetVestingCoins(blockTime), amount) } //----------------------------------------------------------------------------- diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 953e02539fd0..27ced726812e 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -14,6 +14,7 @@ const ( costSubtractCoins sdk.Gas = 10 costAddCoins sdk.Gas = 10 costGetAccount sdk.Gas = 10 + costSetAccount sdk.Gas = 10 ) //----------------------------------------------------------------------------- @@ -202,6 +203,11 @@ func getAccount(ctx sdk.Context, ak auth.AccountKeeper, addr sdk.AccAddress) aut return ak.GetAccount(ctx, addr) } +func setAccount(ctx sdk.Context, ak auth.AccountKeeper, acc auth.Account) { + ctx.GasMeter().ConsumeGas(costSetAccount, "setAccount") + ak.SetAccount(ctx, acc) +} + // subtractCoins subtracts amt coins from an account with the given address addr. // // CONTRACT: If the account is a vesting account, the amount has to be spendable. @@ -304,10 +310,8 @@ func delegateCoins( return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) } - newCoins := acc.TrackDelegation(ctx.BlockHeader().Time, amt) - if err := setCoins(ctx, ak, addr, newCoins); err != nil { - return nil, err - } + acc.TrackDelegation(ctx.BlockHeader().Time, amt) + setAccount(ctx, ak, acc) return sdk.NewTags( sdk.TagAction, TagActionDelegateCoins, @@ -326,10 +330,8 @@ func undelegateCoins( return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) } - newCoins := acc.TrackUndelegation(amt) - if err := setCoins(ctx, ak, addr, newCoins); err != nil { - return nil, err - } + acc.TrackUndelegation(amt) + setAccount(ctx, ak, acc) return sdk.NewTags( sdk.TagAction, TagActionUndelegateCoins, From 96c4b5b56bd86dfc95706c1966bf3f7a941d1f0c Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 5 Dec 2018 15:14:15 -0500 Subject: [PATCH 67/76] Add TODOs for binary search --- types/coin.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/types/coin.go b/types/coin.go index c1f44348f767..670306e6177b 100644 --- a/types/coin.go +++ b/types/coin.go @@ -168,6 +168,8 @@ func (coins Coins) AddCoinByDenom(other Coin) Coins { return Coins{other} } + // TODO: Perform binary search on coins to improve runtime performance + // (i.e. implement a generic binary coin search on Coins) for i, coin := range res { if coin.Denom == other.Denom { res[i] = coin.Plus(other) @@ -257,6 +259,8 @@ func (coins Coins) SubCoinByDenom(other Coin) Coins { return Coins{other} } + // TODO: Perform binary search on coins to improve runtime performance + // (i.e. implement a generic binary coin search on Coins) for i, coin := range res { if coin.Denom == other.Denom { otherNeg := Coin{other.Denom, other.Amount.Neg()} From 31e208a5110e5bf8c8f795c9841d09f11c2f48eb Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Dec 2018 10:47:34 -0500 Subject: [PATCH 68/76] Panic during add/sub coin by denom when denom is not found --- types/coin.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/types/coin.go b/types/coin.go index 670306e6177b..c746d39468ab 100644 --- a/types/coin.go +++ b/types/coin.go @@ -173,11 +173,11 @@ func (coins Coins) AddCoinByDenom(other Coin) Coins { for i, coin := range res { if coin.Denom == other.Denom { res[i] = coin.Plus(other) - break + return res } } - return res + panic(fmt.Sprintf("coin denom %s not found in coins", other.Denom)) } // Plus adds two sets of coins. @@ -273,15 +273,15 @@ func (coins Coins) SubCoinByDenom(other Coin) Coins { panic("negative coin amount") } - break + if len(res) == 0 { + return nil + } else { + return res + } } } - if len(res) == 0 { - return nil - } - - return res + panic(fmt.Sprintf("coin denom %s not found in coins", other.Denom)) } // Minus subtracts a set of coins from another. From b31f288eb4e2c412d261f2ae43cfda83a69784a3 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Dec 2018 10:49:10 -0500 Subject: [PATCH 69/76] Update godoc and unit tests --- types/coin.go | 6 ++++-- types/coin_test.go | 22 ++++++++++++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/types/coin.go b/types/coin.go index c746d39468ab..8e184d2e209f 100644 --- a/types/coin.go +++ b/types/coin.go @@ -160,7 +160,8 @@ func (coins Coins) IsValid() bool { // AddCoinByDenom looks for a matching coin in coins that has the same denom as // other. Upon matching, the other coin is added to the matching coin. If the -// coins are empty, the coin is simply added to the empty set. +// coins are empty, the coin is simply added to the empty set. If the denom of +// the coin to be added does not exist in coins, the function will panic. func (coins Coins) AddCoinByDenom(other Coin) Coins { res := copyCoins(coins) @@ -251,7 +252,8 @@ func (coins Coins) safePlus(coinsB Coins) Coins { // SubCoinByDenom looks for a matching coin in coins that has the same denom as // other. Upon matching, the other coin is subtracted from the matching coin. If // the resulting coin is zero, it is removed. If the coins are empty, the coin -// is simply added to the empty set. +// is simply added to the empty set. If the denom of the coin to be subtracted +// does not exist in coins, the function will panic. func (coins Coins) SubCoinByDenom(other Coin) Coins { res := copyCoins(coins) diff --git a/types/coin_test.go b/types/coin_test.go index 9596d5aab1bf..337561c45c65 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -489,30 +489,40 @@ func TestAmountOf(t *testing.T) { func TestCoinsAddCoin(t *testing.T) { testCases := []struct { - coins Coins - other Coin - expected Coins + coins Coins + other Coin + expected Coins + shouldPanic bool }{ { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, NewInt64Coin("A", 5), Coins{NewInt64Coin("A", 15), NewInt64Coin("B", 5)}, + false, }, { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, NewInt64Coin("C", 5), Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, + true, }, { Coins{}, NewInt64Coin("C", 5), Coins{NewInt64Coin("C", 5)}, + false, }, } for i, tc := range testCases { - coins := tc.coins.AddCoinByDenom(tc.other) - require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) + if tc.shouldPanic { + require.Panics(t, func() { + tc.coins.AddCoinByDenom(tc.other) + }) + } else { + coins := tc.coins.AddCoinByDenom(tc.other) + require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) + } } } @@ -533,7 +543,7 @@ func TestCoinsSubCoin(t *testing.T) { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, NewInt64Coin("C", 5), Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, - false, + true, }, { Coins{NewInt64Coin("A", 10), NewInt64Coin("B", 5)}, From 211ab220fdbbc0eaf77f7817b5449710e0320222 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 6 Dec 2018 11:36:32 -0500 Subject: [PATCH 70/76] Fix linting --- types/coin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/types/coin.go b/types/coin.go index 8e184d2e209f..948582df873d 100644 --- a/types/coin.go +++ b/types/coin.go @@ -277,9 +277,9 @@ func (coins Coins) SubCoinByDenom(other Coin) Coins { if len(res) == 0 { return nil - } else { - return res } + + return res } } From fae2877d55438b15e18e4581b268b9692f89e564 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Jan 2019 14:49:06 -0500 Subject: [PATCH 71/76] Update AddCoinByDenom to not panic --- docs/spec/auth/vesting.md | 2 +- types/coin.go | 5 +++-- types/coin_test.go | 20 +++++--------------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index b25ae7f60f6b..200de925617c 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -107,7 +107,7 @@ Given a vesting account, we define the following in the proceeding operations: - `V'`: The number of `OV` coins that are _vested_ (unlocked). This value is computed on demand and not a per-block basis. - `DV`: The number of delegated _vesting_ coins. It is a variable value. It is stored and modified directly in the vesting account. - `DF`: The number of delegated _vested_ (unlocked) coins. It is a variable value. It is stored and modified directly in the vesting account. -- `BC`: The number of `OV` coins less any coins that are transferred, which can be negative, or delegated (`DV + DF`). It is considered to be balance of the embedded base account. It is stored and modified directly in the vesting account. +- `BC`: The number of `OV` coins less any coins that are transferred (which can be negative or delegated). It is considered to be balance of the embedded base account. It is stored and modified directly in the vesting account. ### Determining Vesting & Vested Amounts diff --git a/types/coin.go b/types/coin.go index 6554a4bab0c3..3bd284d0ccfc 100644 --- a/types/coin.go +++ b/types/coin.go @@ -175,7 +175,8 @@ func (coins Coins) IsValid() bool { // AddCoinByDenom looks for a matching coin in coins that has the same denom as // other. Upon matching, the other coin is added to the matching coin. If the // coins are empty, the coin is simply added to the empty set. If the denom of -// the coin to be added does not exist in coins, the function will panic. +// the other coin to be added does not exist in coins, the function returns the +// original coins. func (coins Coins) AddCoinByDenom(other Coin) Coins { res := copyCoins(coins) @@ -192,7 +193,7 @@ func (coins Coins) AddCoinByDenom(other Coin) Coins { } } - panic(fmt.Sprintf("coin denom %s not found in coins", other.Denom)) + return res } // Plus adds two sets of coins. diff --git a/types/coin_test.go b/types/coin_test.go index 93a59a709733..573b11f2662e 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -511,40 +511,30 @@ func TestAmountOf(t *testing.T) { func TestCoinsAddCoin(t *testing.T) { testCases := []struct { - coins Coins - other Coin - expected Coins - shouldPanic bool + coins Coins + other Coin + expected Coins }{ { Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, NewInt64Coin("a", 5), Coins{NewInt64Coin("a", 15), NewInt64Coin("b", 5)}, - false, }, { Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, NewInt64Coin("c", 5), Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - true, }, { Coins{}, NewInt64Coin("c", 5), Coins{NewInt64Coin("c", 5)}, - false, }, } for i, tc := range testCases { - if tc.shouldPanic { - require.Panics(t, func() { - tc.coins.AddCoinByDenom(tc.other) - }) - } else { - coins := tc.coins.AddCoinByDenom(tc.other) - require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) - } + coins := tc.coins.AddCoinByDenom(tc.other) + require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) } } From 0b6039f1fd93c0fcd2227cc05147dcbe8752a6f7 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Jan 2019 15:25:14 -0500 Subject: [PATCH 72/76] Remove TrackDelegation and TrackUndelegation from Account interface --- x/bank/keeper.go | 32 ++++++++++++++++++++++++++++++-- x/bank/keeper_test.go | 6 +++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/x/bank/keeper.go b/x/bank/keeper.go index 44512d189cd4..499b43e59499 100644 --- a/x/bank/keeper.go +++ b/x/bank/keeper.go @@ -2,6 +2,7 @@ package bank import ( "fmt" + "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/auth" @@ -157,6 +158,7 @@ func (keeper BaseViewKeeper) HasCoins(ctx sdk.Context, addr sdk.AccAddress, amt } //----------------------------------------------------------------------------- +// Auxiliary func getCoins(ctx sdk.Context, am auth.AccountKeeper, addr sdk.AccAddress) sdk.Coins { acc := am.GetAccount(ctx, addr) @@ -301,7 +303,10 @@ func delegateCoins( return nil, sdk.ErrInsufficientCoins(fmt.Sprintf("%s < %s", oldCoins, amt)) } - acc.TrackDelegation(ctx.BlockHeader().Time, amt) + if err := trackDelegation(acc, ctx.BlockHeader().Time, amt); err != nil { + return nil, sdk.ErrInternal(fmt.Sprintf("failed to track delegation: %v", err)) + } + setAccount(ctx, ak, acc) return sdk.NewTags( @@ -319,7 +324,10 @@ func undelegateCoins( return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) } - acc.TrackUndelegation(amt) + if err := trackUndelegation(acc, amt); err != nil { + return nil, sdk.ErrInternal(fmt.Sprintf("failed to track undelegation: %v", err)) + } + setAccount(ctx, ak, acc) return sdk.NewTags( @@ -327,3 +335,23 @@ func undelegateCoins( sdk.TagDelegator, []byte(addr.String()), ), nil } + +func trackDelegation(acc auth.Account, blockTime time.Time, amount sdk.Coins) error { + vacc, ok := acc.(auth.VestingAccount) + if ok { + vacc.TrackDelegation(blockTime, amount) + return nil + } + + return acc.SetCoins(acc.GetCoins().Minus(amount)) +} + +func trackUndelegation(acc auth.Account, amount sdk.Coins) error { + vacc, ok := acc.(auth.VestingAccount) + if ok { + vacc.TrackUndelegation(amount) + return nil + } + + return acc.SetCoins(acc.GetCoins().Plus(amount)) +} diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go index 0f844c52473a..0473578f47fd 100644 --- a/x/bank/keeper_test.go +++ b/x/bank/keeper_test.go @@ -318,12 +318,14 @@ func TestUndelegateCoins(t *testing.T) { ctx = ctx.WithBlockTime(now.Add(12 * time.Hour)) - // require the ability for a non-vesting account to undelegate + // require the ability for a non-vesting account to delegate _, err := bankKeeper.DelegateCoins(ctx, addr2, delCoins) require.NoError(t, err) + // require the ability for a non-vesting account to undelegate _, err = bankKeeper.UndelegateCoins(ctx, addr2, delCoins) require.NoError(t, err) + acc = input.ak.GetAccount(ctx, addr2) require.Equal(t, origCoins, acc.GetCoins()) @@ -331,8 +333,10 @@ func TestUndelegateCoins(t *testing.T) { _, err = bankKeeper.DelegateCoins(ctx, addr1, delCoins) require.NoError(t, err) + // require the ability for a vesting account to undelegate _, err = bankKeeper.UndelegateCoins(ctx, addr1, delCoins) require.NoError(t, err) + vacc = input.ak.GetAccount(ctx, addr1).(*auth.ContinuousVestingAccount) require.Equal(t, origCoins, vacc.GetCoins()) } From cf8b5c772cfec1ad036010b4d680611a40e82d5e Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Jan 2019 15:26:22 -0500 Subject: [PATCH 73/76] Remove TrackDelegation and TrackUndelegation from Account interface --- x/auth/account.go | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index 7c2a066cc860..887b9225c27e 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -35,17 +35,17 @@ type Account interface { // Calculates the amount of coins that can be sent to other accounts given // the current time. SpendableCoins(blockTime time.Time) sdk.Coins - - // Delegation and undelegation accounting that returns the resulting base - // coins amount. - TrackDelegation(blockTime time.Time, amount sdk.Coins) - TrackUndelegation(amount sdk.Coins) } // 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 } @@ -146,18 +146,6 @@ func (acc *BaseAccount) SpendableCoins(_ time.Time) sdk.Coins { return acc.GetCoins() } -// TrackDelegation performs delegation accounting. For a base account it simply -// sets the base coins minus the desired delegation amount. -func (acc *BaseAccount) TrackDelegation(blockTime time.Time, amount sdk.Coins) { - acc.Coins = acc.Coins.Minus(amount) -} - -// TrackUndelegation performs undelegation accounting. For a base account it -// simply sets the base coins plus the undelegation amount. -func (acc *BaseAccount) TrackUndelegation(amount sdk.Coins) { - acc.Coins = acc.Coins.Plus(amount) -} - //----------------------------------------------------------------------------- // Base Vesting Account From 4f70eece0a5db262cae2771afbb858a91401bda5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Jan 2019 15:35:23 -0500 Subject: [PATCH 74/76] Revise vesting spec - Fix any glaring grammatical errors - Update to reflect latest PR updates --- docs/spec/auth/vesting.md | 55 +++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 28 deletions(-) diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index 200de925617c..75e458da0621 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -22,15 +22,13 @@ ## Intro and Requirements -This paper specifies vesting account implementation for the Cosmos Hub. +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` coins and a vesting end time `T`. +during genesis with a starting balance `X` and a vesting end time `T`. -The owner of this account should be able to delegate to validators -and vote with locked coins, however they cannot send locked coins to other -accounts until those coins have been unlocked. When it comes to governance, it -is yet undefined if we want to allow a vesting account to be able to deposit -vesting coins into proposals. +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. In addition, a vesting account vests all of its coin denominations at the same rate. This may be subject to change. @@ -49,6 +47,11 @@ type VestingAccount interface { GetVestedCoins(Time) Coins GetVestingCoins(Time) Coins + + // Delegation and undelegation accounting that returns the resulting base + // coins amount. + TrackDelegation(Time, Coins) + TrackUndelegation(Coins) } // BaseVestingAccount implements the VestingAccount interface. It contains all @@ -90,11 +93,6 @@ type Account interface { // Calculates the amount of coins that can be sent to other accounts given // the current time. SpendableCoins(Time) Coins - - // Delegation and undelegation accounting that returns the resulting base - // coins amount. - TrackDelegation(Time, Coins) - TrackUndelegation(Coins) } ``` @@ -112,7 +110,7 @@ Given a vesting account, we define the following in the proceeding operations: ### Determining Vesting & Vested Amounts It is important to note that these values are computed on demand and not on a -mandatory per-block basis. +mandatory per-block basis (e.g. `BeginBlocker` or `EndBlocker`). #### Continuously Vesting Accounts @@ -201,6 +199,7 @@ func SendCoins(t Time, from Account, to Account, amount Coins) { from.SetCoins(bc - amount) to.SetCoins(amount) + // save accounts... } ``` @@ -267,7 +266,7 @@ func (cva ContinuousVestingAccount) TrackUndelegation(amount Coins) { ``` **Note**: If a delegation is slashed, the continuous vesting account will end up -with excess an `DV` amount, even after all its coins have vested. This is because +with an excess `DV` amount, even after all its coins have vested. This is because undelegating free coins are prioritized. #### Keepers/Handlers @@ -302,8 +301,8 @@ See the above specification for full implementation details. ## Initializing at Genesis -To initialize both vesting accounts and base accounts, the `GenesisAccount` -struct will include an `EndTime`. Accounts meant to be of type `BaseAccount` will +To initialize both vesting and base accounts, the `GenesisAccount` struct will +include an `EndTime`. Accounts meant to be of type `BaseAccount` will have `EndTime = 0`. The `initChainer` method will parse the GenesisAccount into BaseAccounts and VestingAccounts as appropriate. @@ -363,30 +362,30 @@ V' = 0 ``` 1. Immediately receives 1 coin - ``` + ```text BC = 11 ``` 2. Time passes, 2 coins vest - ``` + ```text V = 8 V' = 2 ``` 3. Delegates 4 coins to validator A - ``` + ```text DV = 4 BC = 7 ``` 4. Sends 3 coins - ``` + ```text BC = 4 ``` 5. More time passes, 2 more coins vest - ``` + ```text V = 6 V' = 4 ``` 6. Sends 2 coins. At this point the account cannot send anymore until further coins vest or it receives additional coins. It can still however, delegate. - ``` + ```text BC = 2 ``` @@ -395,34 +394,34 @@ V' = 0 Same initial starting conditions as the simple example. 1. Time passes, 5 coins vest - ``` + ```text V = 5 V' = 5 ``` 2. Delegate 5 coins to validator A - ``` + ```text DV = 5 BC = 5 ``` 3. Delegate 5 coins to validator B - ``` + ```text DF = 5 BC = 0 ``` 4. Validator A gets slashed by 50%, making the delegation to A now worth 2.5 coins 5. Undelegate from validator A (2.5 coins) - ``` + ```text DF = 5 - 2.5 = 2.5 BC = 0 + 2.5 = 2.5 ``` 6. Undelegate from validator B (5 coins). The account at this point can only send 2.5 coins unless it receives more coins or until more coins vest. It can still however, delegate. - ``` + ```text DV = 5 - 2.5 = 2.5 DF = 2.5 - 2.5 = 0 BC = 2.5 + 5 = 7.5 ``` -Notice how we have an excess amount of `DV`. + Notice how we have an excess amount of `DV`. ## Glossary From 39b03874354ac9035c8b26680d0b072b26b5c7b5 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Fri, 11 Jan 2019 15:39:55 -0500 Subject: [PATCH 75/76] Do not add coin to empty set on Add/SubCoinsByDenom --- types/coin.go | 19 +++++-------------- types/coin_test.go | 6 +++--- x/auth/account.go | 42 +++++++++++++++++++++++++++++++++++------- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/types/coin.go b/types/coin.go index 3bd284d0ccfc..b8a75ec02de8 100644 --- a/types/coin.go +++ b/types/coin.go @@ -174,16 +174,12 @@ func (coins Coins) IsValid() bool { // AddCoinByDenom looks for a matching coin in coins that has the same denom as // other. Upon matching, the other coin is added to the matching coin. If the -// coins are empty, the coin is simply added to the empty set. If the denom of -// the other coin to be added does not exist in coins, the function returns the -// original coins. +// coins are empty, then the coin will not be added. If the denom of the other +// coin to be added does not exist in coins, the function returns the original +// coins. func (coins Coins) AddCoinByDenom(other Coin) Coins { res := copyCoins(coins) - if len(res) == 0 { - return Coins{other} - } - // TODO: Perform binary search on coins to improve runtime performance // (i.e. implement a generic binary coin search on Coins) for i, coin := range res { @@ -266,16 +262,11 @@ func (coins Coins) safePlus(coinsB Coins) Coins { // SubCoinByDenom looks for a matching coin in coins that has the same denom as // other. Upon matching, the other coin is subtracted from the matching coin. If -// the resulting coin is zero, it is removed. If the coins are empty, the coin -// is simply added to the empty set. If the denom of the coin to be subtracted -// does not exist in coins, the function will panic. +// the resulting coin is zero, it is removed. If the denom of the coin to be +// subtracted does not exist in coins, the function will panic. func (coins Coins) SubCoinByDenom(other Coin) Coins { res := copyCoins(coins) - if len(res) == 0 { - return Coins{other} - } - // TODO: Perform binary search on coins to improve runtime performance // (i.e. implement a generic binary coin search on Coins) for i, coin := range res { diff --git a/types/coin_test.go b/types/coin_test.go index 573b11f2662e..428946058227 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -528,7 +528,7 @@ func TestCoinsAddCoin(t *testing.T) { { Coins{}, NewInt64Coin("c", 5), - Coins{NewInt64Coin("c", 5)}, + Coins{}, }, } @@ -572,8 +572,8 @@ func TestCoinsSubCoin(t *testing.T) { { Coins{}, NewInt64Coin("c", 5), - Coins{NewInt64Coin("c", 5)}, - false, + Coins{}, + true, }, { Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, diff --git a/x/auth/account.go b/x/auth/account.go index 887b9225c27e..920b6dea68f5 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -197,7 +197,11 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { spendableCoin := sdk.NewCoin(coin.Denom, min) if !spendableCoin.IsZero() { - spendableCoins = spendableCoins.AddCoinByDenom(spendableCoin) + if spendableCoins.Empty() { + spendableCoins = sdk.Coins{spendableCoin} + } else { + spendableCoins = spendableCoins.AddCoinByDenom(spendableCoin) + } } } @@ -255,15 +259,27 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - bva.DelegatedVesting = bva.DelegatedVesting.AddCoinByDenom(xCoin) + if bva.DelegatedVesting.Empty() { + bva.DelegatedVesting = sdk.Coins{xCoin} + } else { + bva.DelegatedVesting = bva.DelegatedVesting.AddCoinByDenom(xCoin) + } } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - bva.DelegatedFree = bva.DelegatedFree.AddCoinByDenom(yCoin) + if bva.DelegatedFree.Empty() { + bva.DelegatedFree = sdk.Coins{yCoin} + } else { + bva.DelegatedFree = bva.DelegatedFree.AddCoinByDenom(yCoin) + } } - bva.Coins = bva.Coins.SubCoinByDenom(coin) + if bva.Coins.Empty() { + bva.Coins = sdk.Coins{coin} + } else { + bva.Coins = bva.Coins.SubCoinByDenom(coin) + } } } @@ -298,15 +314,27 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - bva.DelegatedFree = bva.DelegatedFree.SubCoinByDenom(xCoin) + if bva.DelegatedFree.Empty() { + bva.DelegatedFree = sdk.Coins{xCoin} + } else { + bva.DelegatedFree = bva.DelegatedFree.SubCoinByDenom(xCoin) + } } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - bva.DelegatedVesting = bva.DelegatedVesting.SubCoinByDenom(yCoin) + if bva.DelegatedVesting.Empty() { + bva.DelegatedVesting = sdk.Coins{yCoin} + } else { + bva.DelegatedVesting = bva.DelegatedVesting.SubCoinByDenom(yCoin) + } } - bva.Coins = bva.Coins.AddCoinByDenom(coin) + if bva.Coins.Empty() { + bva.Coins = sdk.Coins{coin} + } else { + bva.Coins = bva.Coins.AddCoinByDenom(coin) + } } } From ab385c10f99917a9e4622bba62c21be578c097ca Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 14 Jan 2019 10:34:50 -0500 Subject: [PATCH 76/76] Remove Add/SubCoinsByDenom in favor of Plus/Minus --- types/coin.go | 52 ---------------------------- types/coin_test.go | 86 ---------------------------------------------- x/auth/account.go | 42 ++++------------------ 3 files changed, 7 insertions(+), 173 deletions(-) diff --git a/types/coin.go b/types/coin.go index dfed20165f91..21fa291f89ca 100644 --- a/types/coin.go +++ b/types/coin.go @@ -172,26 +172,6 @@ func (coins Coins) IsValid() bool { } } -// AddCoinByDenom looks for a matching coin in coins that has the same denom as -// other. Upon matching, the other coin is added to the matching coin. If the -// coins are empty, then the coin will not be added. If the denom of the other -// coin to be added does not exist in coins, the function returns the original -// coins. -func (coins Coins) AddCoinByDenom(other Coin) Coins { - res := copyCoins(coins) - - // TODO: Perform binary search on coins to improve runtime performance - // (i.e. implement a generic binary coin search on Coins) - for i, coin := range res { - if coin.Denom == other.Denom { - res[i] = coin.Plus(other) - return res - } - } - - return res -} - // Plus adds two sets of coins. // // e.g. @@ -260,38 +240,6 @@ func (coins Coins) safePlus(coinsB Coins) Coins { } } -// SubCoinByDenom looks for a matching coin in coins that has the same denom as -// other. Upon matching, the other coin is subtracted from the matching coin. If -// the resulting coin is zero, it is removed. If the denom of the coin to be -// subtracted does not exist in coins, the function will panic. -func (coins Coins) SubCoinByDenom(other Coin) Coins { - res := copyCoins(coins) - - // TODO: Perform binary search on coins to improve runtime performance - // (i.e. implement a generic binary coin search on Coins) - for i, coin := range res { - if coin.Denom == other.Denom { - otherNeg := Coin{other.Denom, other.Amount.Neg()} - - res[i] = coin.Plus(otherNeg) - if res[i].IsZero() { - // remove resulting zero coin - res = append(res[:i], res[i+1:]...) - } else if res[i].IsNegative() { - panic("negative coin amount") - } - - if len(res) == 0 { - return nil - } - - return res - } - } - - panic(fmt.Sprintf("coin denom %s not found in coins", other.Denom)) -} - // Minus subtracts a set of coins from another. // // e.g. diff --git a/types/coin_test.go b/types/coin_test.go index aa92c1199ebd..fb2d67acac78 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -524,89 +524,3 @@ func TestAmountOf(t *testing.T) { assert.Panics(t, func() { cases[0].coins.AmountOf("Invalid") }) } - -func TestCoinsAddCoin(t *testing.T) { - testCases := []struct { - coins Coins - other Coin - expected Coins - }{ - { - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - NewInt64Coin("a", 5), - Coins{NewInt64Coin("a", 15), NewInt64Coin("b", 5)}, - }, - { - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - NewInt64Coin("c", 5), - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - }, - { - Coins{}, - NewInt64Coin("c", 5), - Coins{}, - }, - } - - for i, tc := range testCases { - coins := tc.coins.AddCoinByDenom(tc.other) - require.Equal(t, tc.expected, coins, "invalid coins after adding a coin; tc #%d", i) - } -} - -func TestCoinsSubCoin(t *testing.T) { - testCases := []struct { - coins Coins - other Coin - expected Coins - shouldPanic bool - }{ - { - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - NewInt64Coin("a", 5), - Coins{NewInt64Coin("a", 5), NewInt64Coin("b", 5)}, - false, - }, - { - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - NewInt64Coin("c", 5), - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - true, - }, - { - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - NewInt64Coin("a", 10), - Coins{NewInt64Coin("b", 5)}, - false, - }, - { - Coins{NewInt64Coin("a", 10)}, - NewInt64Coin("a", 10), - nil, - false, - }, - { - Coins{}, - NewInt64Coin("c", 5), - Coins{}, - true, - }, - { - Coins{NewInt64Coin("a", 10), NewInt64Coin("b", 5)}, - NewInt64Coin("a", 15), - Coins{}, - true, - }, - } - - for i, tc := range testCases { - if tc.shouldPanic { - require.Panics(t, func() { - tc.coins.SubCoinByDenom(tc.other) - }) - } else { - coins := tc.coins.SubCoinByDenom(tc.other) - require.Equal(t, tc.expected, coins, "invalid coins after subtracting a coin; tc #%d", i) - } - } -} diff --git a/x/auth/account.go b/x/auth/account.go index 920b6dea68f5..bc279a579eb2 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -197,11 +197,7 @@ func (bva BaseVestingAccount) spendableCoins(vestingCoins sdk.Coins) sdk.Coins { spendableCoin := sdk.NewCoin(coin.Denom, min) if !spendableCoin.IsZero() { - if spendableCoins.Empty() { - spendableCoins = sdk.Coins{spendableCoin} - } else { - spendableCoins = spendableCoins.AddCoinByDenom(spendableCoin) - } + spendableCoins = spendableCoins.Plus(sdk.Coins{spendableCoin}) } } @@ -259,27 +255,15 @@ func (bva *BaseVestingAccount) trackDelegation(vestingCoins, amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - if bva.DelegatedVesting.Empty() { - bva.DelegatedVesting = sdk.Coins{xCoin} - } else { - bva.DelegatedVesting = bva.DelegatedVesting.AddCoinByDenom(xCoin) - } + bva.DelegatedVesting = bva.DelegatedVesting.Plus(sdk.Coins{xCoin}) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - if bva.DelegatedFree.Empty() { - bva.DelegatedFree = sdk.Coins{yCoin} - } else { - bva.DelegatedFree = bva.DelegatedFree.AddCoinByDenom(yCoin) - } + bva.DelegatedFree = bva.DelegatedFree.Plus(sdk.Coins{yCoin}) } - if bva.Coins.Empty() { - bva.Coins = sdk.Coins{coin} - } else { - bva.Coins = bva.Coins.SubCoinByDenom(coin) - } + bva.Coins = bva.Coins.Minus(sdk.Coins{coin}) } } @@ -314,27 +298,15 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { if !x.IsZero() { xCoin := sdk.NewCoin(coin.Denom, x) - if bva.DelegatedFree.Empty() { - bva.DelegatedFree = sdk.Coins{xCoin} - } else { - bva.DelegatedFree = bva.DelegatedFree.SubCoinByDenom(xCoin) - } + bva.DelegatedFree = bva.DelegatedFree.Minus(sdk.Coins{xCoin}) } if !y.IsZero() { yCoin := sdk.NewCoin(coin.Denom, y) - if bva.DelegatedVesting.Empty() { - bva.DelegatedVesting = sdk.Coins{yCoin} - } else { - bva.DelegatedVesting = bva.DelegatedVesting.SubCoinByDenom(yCoin) - } + bva.DelegatedVesting = bva.DelegatedVesting.Minus(sdk.Coins{yCoin}) } - if bva.Coins.Empty() { - bva.Coins = sdk.Coins{coin} - } else { - bva.Coins = bva.Coins.AddCoinByDenom(coin) - } + bva.Coins = bva.Coins.Plus(sdk.Coins{coin}) } }