From f5e6685dfd53bc0dfdb8cf874710131c3e0b0dbc Mon Sep 17 00:00:00 2001 From: Zaki Manian <zaki@manian.org> Date: Thu, 24 Jan 2019 08:38:27 -0800 Subject: [PATCH 1/4] Merge Pull Request #3241: Add Fee & Gas Spec --- docs/spec/auth/README.md | 11 ++++++----- docs/spec/auth/gas_fees.md | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 docs/spec/auth/gas_fees.md diff --git a/docs/spec/auth/README.md b/docs/spec/auth/README.md index 6db6db226c06..be6647a801e9 100644 --- a/docs/spec/auth/README.md +++ b/docs/spec/auth/README.md @@ -16,14 +16,15 @@ This module will be used in the Cosmos Hub. 1. **[State](state.md)** 1. [Accounts](state.md#accounts) 1. [Account Interface](state.md#account-interface) - 1. [Base Account](state.md#baseaccount) - 1. [Vesting Account](state.md#vestingaccount) + 2. [Base Account](state.md#baseaccount) + 3. [Vesting Account](state.md#vestingaccount) 1. **[Types](types.md)** 1. [StdFee](types.md#stdfee) - 1. [StdSignature](types.md#stdsignature) - 1. [StdTx](types.md#stdtx) - 1. [StdSignDoc](types.md#stdsigndoc) + 2. [StdSignature](types.md#stdsignature) + 3. [StdTx](types.md#stdtx) + 4. [StdSignDoc](types.md#stdsigndoc) 1. **[Keepers](keepers.md)** 1. [AccountKeeper](keepers.md#account-keeper) 1. **[Handlers](handlers.md)** 1. [Ante Handler](handlers.md#ante-handler) +1. **[Gas & Fees](gas_fees.md)** diff --git a/docs/spec/auth/gas_fees.md b/docs/spec/auth/gas_fees.md new file mode 100644 index 000000000000..fbe21039461a --- /dev/null +++ b/docs/spec/auth/gas_fees.md @@ -0,0 +1,28 @@ +# Gas & Fees + +Fees serve two purposes for an operator of the network. + +Fees limit the growth of the state stored by every full node and allow for +general purpose censorship of transactions of little economic value. Fees +are best suited as an anti-spam mechanism where validators are disinterested in +the use of the network and identities of users. + +Fees are determined by the gas limits and gas prices transactions provide. +Operators should set minimum gas prices when starting their nodes. They must set +the unit costs of gas in each token denomination they wish to support: + +`gaiad start ... --minimum-gas-prices=0.00001steak,0.05photinos` + +When adding transactions to mempool or gossipping transactions, validators check +if the transaction's gas prices, which are determined by the provided fees, meet +any of the validator's minimum gas prices. In other words, a transaction must +provide a fee of at least one denomination that matches a validator's minimum +gas price. + +Tendermint does not currently provide fee based mempool prioritization, and fee +based mempool filtering is local to node and not part of consensus. But with +minimum gas prices set, such a mechanism could be implemented by node operators. + +Because the market value for tokens will fluctuate, validators are expected to +dynamically adjust their minimum gas prices to a level that would encourage the +use of the network. From 168034746d4eb03f37819c36483e00bc49dae645 Mon Sep 17 00:00:00 2001 From: Alessio Treglia <quadrispro@ubuntu.com> Date: Thu, 24 Jan 2019 19:29:24 +0000 Subject: [PATCH 2/4] Merge PR #3374: Install and build-linux depend on vendor-deps --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index cb1281168910..ec02833dbb3f 100644 --- a/Makefile +++ b/Makefile @@ -62,13 +62,13 @@ else go build $(BUILD_FLAGS) -o build/gaiakeyutil ./cmd/gaia/cmd/gaiakeyutil endif -build-linux: +build-linux: vendor-deps LEDGER_ENABLED=false GOOS=linux GOARCH=amd64 $(MAKE) build update_gaia_lite_docs: @statik -src=client/lcd/swagger-ui -dest=client/lcd -f -install: check-ledger update_gaia_lite_docs +install: vendor-deps check-ledger update_gaia_lite_docs go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiad go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiacli go install $(BUILD_FLAGS) ./cmd/gaia/cmd/gaiareplay From ccfcee8b72082a62d64fc4de64fc09d6df1efb2b Mon Sep 17 00:00:00 2001 From: Christopher Goes <cwgoes@pluranimity.org> Date: Thu, 24 Jan 2019 20:44:06 +0100 Subject: [PATCH 3/4] Merge PR #3381: Add vendor-deps to .gitignore for now --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6c936c9e7472..9f480652448d 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,7 @@ # Build vendor +vendor-deps .vendor-new build tools/bin/* From af60c75dd3f642560f2afc1a7cd6f9398354e6ed Mon Sep 17 00:00:00 2001 From: Jack Zampolin <jack.zampolin@gmail.com> Date: Thu, 24 Jan 2019 13:01:32 -0800 Subject: [PATCH 4/4] Merge PR #3352: Reenable simulation tests --- Makefile | 6 ++-- PENDING.md | 1 + cmd/gaia/app/genesis.go | 33 +++++++++++++---- cmd/gaia/app/genesis_test.go | 1 + cmd/gaia/app/sim_test.go | 6 ++-- docs/spec/auth/vesting.md | 15 ++++---- scripts/multisim.sh | 2 +- types/dec_coin.go | 26 ++++++++++++++ types/decimal.go | 27 ++++++++++++++ x/auth/account.go | 45 ++++++++++++++++++------ x/auth/simulation/fake.go | 30 +++++++++++----- x/distribution/keeper/delegation.go | 22 +++++++++--- x/distribution/keeper/delegation_test.go | 16 ++++----- x/distribution/keeper/store.go | 8 +++-- x/distribution/keeper/validator.go | 18 ++++++++-- x/distribution/simulation/invariants.go | 6 ++-- x/staking/genesis.go | 12 +++++-- x/staking/keeper/delegation.go | 12 ++++--- x/staking/keeper/slash.go | 7 +++- 19 files changed, 226 insertions(+), 67 deletions(-) diff --git a/Makefile b/Makefile index ec02833dbb3f..2f8086fd7ae4 100644 --- a/Makefile +++ b/Makefile @@ -156,15 +156,15 @@ test_sim_gaia_fast: test_sim_gaia_import_export: @echo "Running Gaia import/export simulation. This may take several minutes..." - @bash scripts/multisim.sh 50 TestGaiaImportExport + @bash scripts/multisim.sh 50 5 TestGaiaImportExport test_sim_gaia_simulation_after_import: @echo "Running Gaia simulation-after-import. This may take several minutes..." - @bash scripts/multisim.sh 50 TestGaiaSimulationAfterImport + @bash scripts/multisim.sh 50 5 TestGaiaSimulationAfterImport test_sim_gaia_multi_seed: @echo "Running multi-seed Gaia simulation. This may take awhile!" - @bash scripts/multisim.sh 400 TestFullGaiaSimulation + @bash scripts/multisim.sh 400 5 TestFullGaiaSimulation SIM_NUM_BLOCKS ?= 500 SIM_BLOCK_SIZE ?= 200 diff --git a/PENDING.md b/PENDING.md index a39f534b10d8..356ea12ecb28 100644 --- a/PENDING.md +++ b/PENDING.md @@ -31,6 +31,7 @@ BREAKING CHANGES * [\#3249\(https://github.com/cosmos/cosmos-sdk/issues/3249) `tendermint`'s `show-validator` and `show-address` `--json` flags removed in favor of `--output-format=json`. * SDK + * [distribution] \#3359 Always round down when calculating rewards-to-be-withdrawn in F1 fee distribution * [staking] \#2513 Validator power type from Dec -> Int * [staking] \#3233 key and value now contain duplicate fields to simplify code * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN. diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index 7c4790de12bf..cfda1c6c488d 100644 --- a/cmd/gaia/app/genesis.go +++ b/cmd/gaia/app/genesis.go @@ -64,9 +64,13 @@ type GenesisAccount struct { Coins sdk.Coins `json:"coins"` Sequence uint64 `json:"sequence_number"` AccountNumber uint64 `json:"account_number"` - Vesting bool `json:"vesting"` - StartTime int64 `json:"start_time"` - EndTime int64 `json:"end_time"` + + // vesting account fields + OriginalVesting sdk.Coins `json:"original_vesting"` // total vesting coins upon initialization + DelegatedFree sdk.Coins `json:"delegated_free"` // delegated vested coins at time of delegation + DelegatedVesting sdk.Coins `json:"delegated_vesting"` // delegated vesting coins at time of delegation + StartTime int64 `json:"start_time"` // vesting start time + EndTime int64 `json:"end_time"` // vesting end time } func NewGenesisAccount(acc *auth.BaseAccount) GenesisAccount { @@ -88,7 +92,9 @@ func NewGenesisAccountI(acc auth.Account) GenesisAccount { vacc, ok := acc.(auth.VestingAccount) if ok { - gacc.Vesting = true + gacc.OriginalVesting = vacc.GetOriginalVesting() + gacc.DelegatedFree = vacc.GetDelegatedFree() + gacc.DelegatedVesting = vacc.GetDelegatedVesting() gacc.StartTime = vacc.GetStartTime() gacc.EndTime = vacc.GetEndTime() } @@ -105,11 +111,24 @@ func (ga *GenesisAccount) ToAccount() auth.Account { Sequence: ga.Sequence, } - if ga.Vesting { + if !ga.OriginalVesting.IsZero() { + baseVestingAcc := &auth.BaseVestingAccount{ + BaseAccount: bacc, + OriginalVesting: ga.OriginalVesting, + DelegatedFree: ga.DelegatedFree, + DelegatedVesting: ga.DelegatedVesting, + EndTime: ga.EndTime, + } + if ga.StartTime != 0 && ga.EndTime != 0 { - return auth.NewContinuousVestingAccount(bacc, ga.StartTime, ga.EndTime) + return &auth.ContinuousVestingAccount{ + BaseVestingAccount: baseVestingAcc, + StartTime: ga.StartTime, + } } else if ga.EndTime != 0 { - return auth.NewDelayedVestingAccount(bacc, ga.EndTime) + return &auth.DelayedVestingAccount{ + BaseVestingAccount: baseVestingAcc, + } } else { panic(fmt.Sprintf("invalid genesis vesting account: %+v", ga)) } diff --git a/cmd/gaia/app/genesis_test.go b/cmd/gaia/app/genesis_test.go index 59872e801863..1383794a3b8a 100644 --- a/cmd/gaia/app/genesis_test.go +++ b/cmd/gaia/app/genesis_test.go @@ -56,6 +56,7 @@ func TestToAccount(t *testing.T) { priv := ed25519.GenPrivKey() addr := sdk.AccAddress(priv.PubKey().Address()) authAcc := auth.NewBaseAccountWithAddress(addr) + authAcc.SetCoins(sdk.Coins{sdk.NewInt64Coin(bondDenom, 150)}) genAcc := NewGenesisAccount(&authAcc) acc := genAcc.ToAccount() require.IsType(t, &auth.BaseAccount{}, acc) diff --git a/cmd/gaia/app/sim_test.go b/cmd/gaia/app/sim_test.go index a5070a8a70dc..b88e809eba6e 100644 --- a/cmd/gaia/app/sim_test.go +++ b/cmd/gaia/app/sim_test.go @@ -425,8 +425,10 @@ func TestGaiaImportExport(t *testing.T) { storeB := ctxB.KVStore(storeKeyB) kvA, kvB, count, equal := sdk.DiffKVStores(storeA, storeB, prefixes) fmt.Printf("Compared %d key/value pairs between %s and %s\n", count, storeKeyA, storeKeyB) - require.True(t, equal, "unequal stores: %s / %s:\nstore A %s (%X) => %s (%X)\nstore B %s (%X) => %s (%X)", - storeKeyA, storeKeyB, kvA.Key, kvA.Key, kvA.Value, kvA.Value, kvB.Key, kvB.Key, kvB.Value, kvB.Value) + require.True(t, equal, + "unequal stores: %s / %s:\nstore A %X => %X\nstore B %X => %X", + storeKeyA, storeKeyB, kvA.Key, kvA.Value, kvB.Key, kvB.Value, + ) } } diff --git a/docs/spec/auth/vesting.md b/docs/spec/auth/vesting.md index f0722e9ebfd1..a0c64fdd5e95 100644 --- a/docs/spec/auth/vesting.md +++ b/docs/spec/auth/vesting.md @@ -316,19 +316,22 @@ and return the correct accounts accordingly based off of these new fields. type GenesisAccount struct { // ... - Vesting bool - EndTime int64 - StartTime int64 + // vesting account fields + OriginalVesting sdk.Coins `json:"original_vesting"` + DelegatedFree sdk.Coins `json:"delegated_free"` + DelegatedVesting sdk.Coins `json:"delegated_vesting"` + StartTime int64 `json:"start_time"` + EndTime int64 `json:"end_time"` } func ToAccount(gacc GenesisAccount) Account { bacc := NewBaseAccount(gacc) - if gacc.Vesting { + if gacc.OriginalVesting > 0 { if ga.StartTime != 0 && ga.EndTime != 0 { - return NewContinuousVestingAccount(bacc, gacc.StartTime, gacc.EndTime) + // return a continuous vesting account } else if ga.EndTime != 0 { - return NewDelayedVestingAccount(bacc, gacc.EndTime) + // return a delayed vesting account } else { // invalid genesis vesting account provided panic() diff --git a/scripts/multisim.sh b/scripts/multisim.sh index dbd1a9e9ac42..e27035983dde 100755 --- a/scripts/multisim.sh +++ b/scripts/multisim.sh @@ -23,7 +23,7 @@ sim() { file="$tmpdir/gaia-simulation-seed-$seed-date-$(date -u +"%Y-%m-%dT%H:%M:%S+00:00").stdout" echo "Writing stdout to $file..." go test ./cmd/gaia/app -run $testname -SimulationEnabled=true -SimulationNumBlocks=$blocks \ - -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=$seed -SimulationPeriod=$period -v -timeout 24h | tee $file + -SimulationVerbose=true -SimulationCommit=true -SimulationSeed=$seed -SimulationPeriod=$period -v -timeout 24h > $file } i=0 diff --git a/types/dec_coin.go b/types/dec_coin.go index b70a1f476592..cc29c921c61b 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -201,6 +201,19 @@ func (coins DecCoins) MulDec(d Dec) DecCoins { return res } +// multiply all the coins by a decimal, truncating +func (coins DecCoins) MulDecTruncate(d Dec) DecCoins { + res := make([]DecCoin, len(coins)) + for i, coin := range coins { + product := DecCoin{ + Denom: coin.Denom, + Amount: coin.Amount.MulTruncate(d), + } + res[i] = product + } + return res +} + // divide all the coins by a decimal func (coins DecCoins) QuoDec(d Dec) DecCoins { res := make([]DecCoin, len(coins)) @@ -214,6 +227,19 @@ func (coins DecCoins) QuoDec(d Dec) DecCoins { return res } +// divide all the coins by a decimal, truncating +func (coins DecCoins) QuoDecTruncate(d Dec) DecCoins { + res := make([]DecCoin, len(coins)) + for i, coin := range coins { + quotient := DecCoin{ + Denom: coin.Denom, + Amount: coin.Amount.QuoTruncate(d), + } + res[i] = quotient + } + return res +} + // returns the amount of a denom from deccoins func (coins DecCoins) AmountOf(denom string) Dec { switch len(coins) { diff --git a/types/decimal.go b/types/decimal.go index 0d1c54af3376..976ddd560e79 100644 --- a/types/decimal.go +++ b/types/decimal.go @@ -228,6 +228,17 @@ func (d Dec) Mul(d2 Dec) Dec { return Dec{chopped} } +// multiplication truncate +func (d Dec) MulTruncate(d2 Dec) Dec { + mul := new(big.Int).Mul(d.Int, d2.Int) + chopped := chopPrecisionAndTruncate(mul) + + if chopped.BitLen() > 255+DecimalPrecisionBits { + panic("Int overflow") + } + return Dec{chopped} +} + // multiplication func (d Dec) MulInt(i Int) Dec { mul := new(big.Int).Mul(d.Int, i.i) @@ -254,6 +265,22 @@ func (d Dec) Quo(d2 Dec) Dec { return Dec{chopped} } +// quotient truncate +func (d Dec) QuoTruncate(d2 Dec) Dec { + + // multiply precision twice + mul := new(big.Int).Mul(d.Int, precisionReuse) + mul.Mul(mul, precisionReuse) + + quo := new(big.Int).Quo(mul, d2.Int) + chopped := chopPrecisionAndTruncate(quo) + + if chopped.BitLen() > 255+DecimalPrecisionBits { + panic("Int overflow") + } + return Dec{chopped} +} + // quotient func (d Dec) QuoInt(i Int) Dec { mul := new(big.Int).Quo(d.Int, i.i) diff --git a/x/auth/account.go b/x/auth/account.go index 24b23549662c..e67e081aea6c 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -55,6 +55,10 @@ type VestingAccount interface { GetStartTime() int64 GetEndTime() int64 + + GetOriginalVesting() sdk.Coins + GetDelegatedFree() sdk.Coins + GetDelegatedVesting() sdk.Coins } // AccountDecoder unmarshals account bytes @@ -179,18 +183,20 @@ type BaseVestingAccount struct { // String implements fmt.Stringer func (bva BaseVestingAccount) String() string { - return fmt.Sprintf(`Vesting Account %s: + return fmt.Sprintf(`Vesting Account: + Address: %s Coins: %s - PubKey: %s AccountNumber: %d Sequence: %d OriginalVesting: %s DelegatedFree: %s DelegatedVesting: %s - EndTime: %d `, bva.Address, bva.Coins, - bva.PubKey.Address(), bva.AccountNumber, bva.Sequence, + EndTime: %d `, + bva.Address, bva.Coins, + bva.AccountNumber, bva.Sequence, bva.OriginalVesting, bva.DelegatedFree, - bva.DelegatedVesting, bva.EndTime) + bva.DelegatedVesting, bva.EndTime, + ) } // spendableCoins returns all the spendable coins for a vesting account given a @@ -342,6 +348,23 @@ func (bva *BaseVestingAccount) TrackUndelegation(amount sdk.Coins) { } } +// GetOriginalVesting returns a vesting account's original vesting amount +func (bva BaseVestingAccount) GetOriginalVesting() sdk.Coins { + return bva.OriginalVesting +} + +// GetDelegatedFree returns a vesting account's delegation amount that is not +// vesting. +func (bva BaseVestingAccount) GetDelegatedFree() sdk.Coins { + return bva.DelegatedFree +} + +// GetDelegatedVesting returns a vesting account's delegation amount that is +// still vesting. +func (bva BaseVestingAccount) GetDelegatedVesting() sdk.Coins { + return bva.DelegatedVesting +} + //----------------------------------------------------------------------------- // Continuous Vesting Account @@ -372,19 +395,21 @@ func NewContinuousVestingAccount( } func (cva ContinuousVestingAccount) String() string { - return fmt.Sprintf(`Vesting Account %s: + return fmt.Sprintf(`Continuous Vesting Account: + Address: %s Coins: %s - PubKey: %s AccountNumber: %d Sequence: %d OriginalVesting: %s DelegatedFree: %s DelegatedVesting: %s StartTime: %d - EndTime: %d `, cva.Address, cva.Coins, - cva.PubKey.Address(), cva.AccountNumber, cva.Sequence, + EndTime: %d `, + cva.Address, cva.Coins, + cva.AccountNumber, cva.Sequence, cva.OriginalVesting, cva.DelegatedFree, - cva.DelegatedVesting, cva.StartTime, cva.EndTime) + cva.DelegatedVesting, cva.StartTime, cva.EndTime, + ) } // GetVestedCoins returns the total number of vested coins. If no coins are vested, diff --git a/x/auth/simulation/fake.go b/x/auth/simulation/fake.go index 222b747ec4a0..c037d74c284e 100644 --- a/x/auth/simulation/fake.go +++ b/x/auth/simulation/fake.go @@ -28,24 +28,36 @@ func SimulateDeductFee(m auth.AccountKeeper, f auth.FeeCollectionKeeper) simulat } denomIndex := r.Intn(len(initCoins)) - amt, err := randPositiveInt(r, initCoins[denomIndex].Amount) + randCoin := initCoins[denomIndex] + + amt, err := randPositiveInt(r, randCoin.Amount) if err != nil { event(fmt.Sprintf("auth/SimulateDeductFee/false")) return action, nil, nil } - coins := sdk.Coins{sdk.NewCoin(initCoins[denomIndex].Denom, amt)} - err = stored.SetCoins(initCoins.Minus(coins)) - if err != nil { - panic(err) + // Create a random fee and verify the fees are within the account's spendable + // balance. + fees := sdk.Coins{sdk.NewCoin(randCoin.Denom, amt)} + spendableCoins := stored.SpendableCoins(ctx.BlockHeader().Time) + if _, hasNeg := spendableCoins.SafeMinus(fees); hasNeg { + event(fmt.Sprintf("auth/SimulateDeductFee/false")) + return action, nil, nil } - m.SetAccount(ctx, stored) - if !coins.IsNotNegative() { - panic("setting negative fees") + + // get the new account balance + newCoins, hasNeg := initCoins.SafeMinus(fees) + if hasNeg { + event(fmt.Sprintf("auth/SimulateDeductFee/false")) + return action, nil, nil } - f.AddCollectedFees(ctx, coins) + if err := stored.SetCoins(newCoins); err != nil { + panic(err) + } + m.SetAccount(ctx, stored) + f.AddCollectedFees(ctx, fees) event(fmt.Sprintf("auth/SimulateDeductFee/true")) action = "TestDeductFee" diff --git a/x/distribution/keeper/delegation.go b/x/distribution/keeper/delegation.go index 3381ee206f22..d75691bdf5af 100644 --- a/x/distribution/keeper/delegation.go +++ b/x/distribution/keeper/delegation.go @@ -11,28 +11,41 @@ func (k Keeper) initializeDelegation(ctx sdk.Context, val sdk.ValAddress, del sd // period has already been incremented - we want to store the period ended by this delegation action previousPeriod := k.GetValidatorCurrentRewards(ctx, val).Period - 1 + // increment reference count for the period we're going to track + k.incrementReferenceCount(ctx, val, previousPeriod) + validator := k.stakingKeeper.Validator(ctx, val) delegation := k.stakingKeeper.Delegation(ctx, del, val) // calculate delegation stake in tokens // we don't store directly, so multiply delegation shares * (tokens per share) - stake := delegation.GetShares().Mul(validator.GetDelegatorShareExRate()) + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + stake := delegation.GetShares().MulTruncate(validator.GetDelegatorShareExRate()) k.SetDelegatorStartingInfo(ctx, val, del, types.NewDelegatorStartingInfo(previousPeriod, stake, uint64(ctx.BlockHeight()))) } // calculate the rewards accrued by a delegation between two periods func (k Keeper) calculateDelegationRewardsBetween(ctx sdk.Context, val sdk.Validator, - startingPeriod, endingPeriod uint64, staking sdk.Dec) (rewards sdk.DecCoins) { + startingPeriod, endingPeriod uint64, stake sdk.Dec) (rewards sdk.DecCoins) { // sanity check if startingPeriod > endingPeriod { panic("startingPeriod cannot be greater than endingPeriod") } + // sanity check + if stake.LT(sdk.ZeroDec()) { + panic("stake should not be negative") + } + // return staking * (ending - starting) starting := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), startingPeriod) ending := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), endingPeriod) difference := ending.CumulativeRewardRatio.Minus(starting.CumulativeRewardRatio) - rewards = difference.MulDec(staking) + if difference.HasNegative() { + panic("negative rewards should not be possible") + } + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + rewards = difference.MulDecTruncate(stake) return } @@ -54,7 +67,8 @@ func (k Keeper) calculateDelegationRewards(ctx sdk.Context, val sdk.Validator, d func(height uint64, event types.ValidatorSlashEvent) (stop bool) { endingPeriod := event.ValidatorPeriod rewards = rewards.Plus(k.calculateDelegationRewardsBetween(ctx, val, startingPeriod, endingPeriod, stake)) - stake = stake.Mul(sdk.OneDec().Sub(event.Fraction)) + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + stake = stake.MulTruncate(sdk.OneDec().Sub(event.Fraction)) startingPeriod = endingPeriod return false }, diff --git a/x/distribution/keeper/delegation_test.go b/x/distribution/keeper/delegation_test.go index 8ec514570a0d..3f601ad32974 100644 --- a/x/distribution/keeper/delegation_test.go +++ b/x/distribution/keeper/delegation_test.go @@ -30,13 +30,13 @@ func TestCalculateRewardsBasic(t *testing.T) { del := sk.Delegation(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1) // historical count should be 2 (once for validator init, once for delegation init) - require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) // end period endingPeriod := k.incrementValidatorPeriod(ctx, val) - // historical count should be 3 (since we ended the period, and haven't yet decremented a reference) - require.Equal(t, uint64(3), k.GetValidatorHistoricalRewardCount(ctx)) + // historical count should be 2 still + require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) // calculate delegation rewards rewards := k.calculateDelegationRewards(ctx, val, del, endingPeriod) @@ -283,13 +283,13 @@ func TestWithdrawDelegationRewardsBasic(t *testing.T) { k.AllocateTokensToValidator(ctx, val, tokens) // historical count should be 2 (initial + latest for delegation) - require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) // withdraw rewards require.Nil(t, k.WithdrawDelegationRewards(ctx, sdk.AccAddress(valOpAddr1), valOpAddr1)) // historical count should still be 2 (added one record, cleared one) - require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) // assert correct balance require.Equal(t, sdk.Coins{{staking.DefaultBondDenom, sdk.NewInt(balance - bond + (initial / 2))}}, ak.GetAccount(ctx, sdk.AccAddress(valOpAddr1)).GetCoins()) @@ -460,14 +460,14 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { k.AllocateTokensToValidator(ctx, val, tokens) // historical count should be 2 (validator init, delegation init) - require.Equal(t, uint64(2), k.GetValidatorHistoricalRewardCount(ctx)) + require.Equal(t, uint64(2), k.GetValidatorHistoricalReferenceCount(ctx)) // second delegation msg2 := staking.NewMsgDelegate(sdk.AccAddress(valOpAddr2), valOpAddr1, sdk.NewCoin(staking.DefaultBondDenom, sdk.NewInt(100))) require.True(t, sh(ctx, msg2).IsOK()) // historical count should be 3 (second delegation init) - require.Equal(t, uint64(3), k.GetValidatorHistoricalRewardCount(ctx)) + require.Equal(t, uint64(3), k.GetValidatorHistoricalReferenceCount(ctx)) // fetch updated validator val = sk.Validator(ctx, valOpAddr1) @@ -486,7 +486,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) { k.WithdrawDelegationRewards(ctx, sdk.AccAddress(valOpAddr2), valOpAddr1) // historical count should be 3 (validator init + two delegations) - require.Equal(t, uint64(3), k.GetValidatorHistoricalRewardCount(ctx)) + require.Equal(t, uint64(3), k.GetValidatorHistoricalReferenceCount(ctx)) // validator withdraws commission k.WithdrawValidatorCommission(ctx, valOpAddr1) diff --git a/x/distribution/keeper/store.go b/x/distribution/keeper/store.go index 7bbf6a13ff2a..ef1f7c783c34 100644 --- a/x/distribution/keeper/store.go +++ b/x/distribution/keeper/store.go @@ -175,13 +175,15 @@ func (k Keeper) DeleteAllValidatorHistoricalRewards(ctx sdk.Context) { } } -// historical record count (used for testcases) -func (k Keeper) GetValidatorHistoricalRewardCount(ctx sdk.Context) (count uint64) { +// historical reference count (used for testcases) +func (k Keeper) GetValidatorHistoricalReferenceCount(ctx sdk.Context) (count uint64) { store := ctx.KVStore(k.storeKey) iter := sdk.KVStorePrefixIterator(store, ValidatorHistoricalRewardsPrefix) defer iter.Close() for ; iter.Valid(); iter.Next() { - count++ + var rewards types.ValidatorHistoricalRewards + k.cdc.MustUnmarshalBinaryLengthPrefixed(iter.Value(), &rewards) + count += uint64(rewards.ReferenceCount) } return } diff --git a/x/distribution/keeper/validator.go b/x/distribution/keeper/validator.go index d7bebb9c7ccb..05326ff12262 100644 --- a/x/distribution/keeper/validator.go +++ b/x/distribution/keeper/validator.go @@ -38,12 +38,16 @@ func (k Keeper) incrementValidatorPeriod(ctx sdk.Context, val sdk.Validator) uin current = sdk.DecCoins{} } else { - current = rewards.Rewards.QuoDec(sdk.NewDecFromInt(val.GetTokens())) + // note: necessary to truncate so we don't allow withdrawing more rewards than owed + current = rewards.Rewards.QuoDecTruncate(sdk.NewDecFromInt(val.GetTokens())) } // fetch historical rewards for last period historical := k.GetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period-1).CumulativeRewardRatio + // decrement reference count + k.decrementReferenceCount(ctx, val.GetOperator(), rewards.Period-1) + // set new historical rewards with reference count of 1 k.SetValidatorHistoricalRewards(ctx, val.GetOperator(), rewards.Period, types.NewValidatorHistoricalRewards(historical.Plus(current), 1)) @@ -56,8 +60,8 @@ func (k Keeper) incrementValidatorPeriod(ctx sdk.Context, val sdk.Validator) uin // increment the reference count for a historical rewards value func (k Keeper) incrementReferenceCount(ctx sdk.Context, valAddr sdk.ValAddress, period uint64) { historical := k.GetValidatorHistoricalRewards(ctx, valAddr, period) - if historical.ReferenceCount > 1 { - panic("reference count should never exceed 1") + if historical.ReferenceCount > 2 { + panic("reference count should never exceed 2") } historical.ReferenceCount++ k.SetValidatorHistoricalRewards(ctx, valAddr, period, historical) @@ -78,6 +82,9 @@ func (k Keeper) decrementReferenceCount(ctx sdk.Context, valAddr sdk.ValAddress, } func (k Keeper) updateValidatorSlashFraction(ctx sdk.Context, valAddr sdk.ValAddress, fraction sdk.Dec) { + if fraction.GT(sdk.OneDec()) { + panic("fraction greater than one") + } height := uint64(ctx.BlockHeight()) currentFraction := sdk.ZeroDec() endedPeriod := k.GetValidatorCurrentRewards(ctx, valAddr).Period - 1 @@ -91,9 +98,14 @@ func (k Keeper) updateValidatorSlashFraction(ctx sdk.Context, valAddr sdk.ValAdd val := k.stakingKeeper.Validator(ctx, valAddr) // increment current period endedPeriod = k.incrementValidatorPeriod(ctx, val) + // increment reference count on period we need to track + k.incrementReferenceCount(ctx, valAddr, endedPeriod) } currentMultiplicand := sdk.OneDec().Sub(currentFraction) newMultiplicand := sdk.OneDec().Sub(fraction) updatedFraction := sdk.OneDec().Sub(currentMultiplicand.Mul(newMultiplicand)) + if updatedFraction.LT(sdk.ZeroDec()) { + panic("negative slash fraction") + } k.SetValidatorSlashEvent(ctx, valAddr, height, types.NewValidatorSlashEvent(endedPeriod, updatedFraction)) } diff --git a/x/distribution/simulation/invariants.go b/x/distribution/simulation/invariants.go index 5b2d0e341d77..2f33d02b2fa4 100644 --- a/x/distribution/simulation/invariants.go +++ b/x/distribution/simulation/invariants.go @@ -85,12 +85,12 @@ func ReferenceCountInvariant(k distr.Keeper, sk staking.Keeper) simulation.Invar return false }) - // one record per validator (zeroeth period), one record per delegation (previous period), one record per slash (previous period) + // one record per validator (last tracked period), one record per delegation (previous period), one record per slash (previous period) expected := valCount + uint64(len(dels)) + slashCount + count := k.GetValidatorHistoricalReferenceCount(ctx) - count := k.GetValidatorHistoricalRewardCount(ctx) if count != expected { - return fmt.Errorf("unexpected number of historical rewards records: expected %v, got %v", expected, count) + return fmt.Errorf("unexpected number of historical rewards records: expected %v (%v vals + %v dels + %v slashes), got %v", expected, valCount, len(dels), slashCount, count) } return nil diff --git a/x/staking/genesis.go b/x/staking/genesis.go index 294d9d40e63b..8fa52c07ac5c 100644 --- a/x/staking/genesis.go +++ b/x/staking/genesis.go @@ -34,7 +34,9 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res [ // Manually set indices for the first time keeper.SetValidatorByConsAddr(ctx, validator) keeper.SetValidatorByPowerIndex(ctx, validator) - keeper.AfterValidatorCreated(ctx, validator.OperatorAddr) + if !data.Exported { + keeper.AfterValidatorCreated(ctx, validator.OperatorAddr) + } // Set timeslice if necessary if validator.Status == sdk.Unbonding { @@ -43,9 +45,13 @@ func InitGenesis(ctx sdk.Context, keeper Keeper, data types.GenesisState) (res [ } for _, delegation := range data.Bonds { - keeper.BeforeDelegationCreated(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) + if !data.Exported { + keeper.BeforeDelegationCreated(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) + } keeper.SetDelegation(ctx, delegation) - keeper.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) + if !data.Exported { + keeper.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) + } } for _, ubd := range data.UnbondingDelegations { diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 553f4097b8ad..5a15b4d03597 100644 --- a/x/staking/keeper/delegation.go +++ b/x/staking/keeper/delegation.go @@ -77,7 +77,6 @@ func (k Keeper) SetDelegation(ctx sdk.Context, delegation types.Delegation) { store := ctx.KVStore(k.storeKey) b := types.MustMarshalDelegation(k.cdc, delegation) store.Set(GetDelegationKey(delegation.DelegatorAddr, delegation.ValidatorAddr), b) - k.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) } // remove a delegation from store @@ -468,6 +467,7 @@ func (k Keeper) Delegate(ctx sdk.Context, delAddr sdk.AccAddress, bondAmt sdk.Co // Update delegation delegation.Shares = delegation.Shares.Add(newShares) k.SetDelegation(ctx, delegation) + k.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) return newShares, nil } @@ -512,6 +512,7 @@ func (k Keeper) unbond(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValA } else { // update the delegation k.SetDelegation(ctx, delegation) + k.AfterDelegationModified(ctx, delegation.DelegatorAddr, delegation.ValidatorAddr) } // remove the coins from the validator @@ -569,10 +570,13 @@ func (k Keeper) Undelegate(ctx sdk.Context, delAddr sdk.AccAddress, // no need to create the ubd object just complete now if completeNow { - _, err := k.bankKeeper.UndelegateCoins(ctx, delAddr, sdk.Coins{balance}) - if err != nil { - return completionTime, err + // track undelegation only when remaining or truncated shares are non-zero + if !balance.IsZero() { + if _, err := k.bankKeeper.UndelegateCoins(ctx, delAddr, sdk.Coins{balance}); err != nil { + return completionTime, err + } } + return completionTime, nil } diff --git a/x/staking/keeper/slash.go b/x/staking/keeper/slash.go index 17c4c644eb78..3ca7ffcad5bc 100644 --- a/x/staking/keeper/slash.go +++ b/x/staking/keeper/slash.go @@ -57,7 +57,12 @@ func (k Keeper) Slash(ctx sdk.Context, consAddr sdk.ConsAddress, infractionHeigh // we need to calculate the *effective* slash fraction for distribution if validator.Tokens.GT(sdk.ZeroInt()) { - k.BeforeValidatorSlashed(ctx, operatorAddress, slashAmountDec.Quo(sdk.NewDecFromInt(validator.Tokens))) + effectiveFraction := slashAmountDec.Quo(sdk.NewDecFromInt(validator.Tokens)) + // possible if power has changed + if effectiveFraction.GT(sdk.OneDec()) { + effectiveFraction = sdk.OneDec() + } + k.BeforeValidatorSlashed(ctx, operatorAddress, effectiveFraction) } // Track remaining slash amount for the validator