From dfd00a661a19df6efe7e0dde96b2fbeb883d1d80 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Tue, 4 Dec 2018 19:17:02 +0100 Subject: [PATCH] Inflation bug fixes (#2982) * PENDING.md; swap BeginBlocker ordering * Calculate inflation every block * Update x/mint spec * Reset distribution info bond height instead --- PENDING.md | 1 + cmd/gaia/app/app.go | 8 +++--- cmd/gaia/app/export.go | 46 ++++++++++++-------------------- cmd/gaia/app/invariants.go | 9 +++++-- docs/spec/mint/begin_block.md | 10 +++---- docs/spec/mint/state.md | 2 -- x/distribution/alias.go | 2 ++ x/distribution/types/dec_coin.go | 10 +++++++ x/mint/abci_app.go | 21 ++++++--------- x/mint/minter.go | 15 +++-------- x/mint/minter_test.go | 41 +++++++++++++++++++++++----- 11 files changed, 93 insertions(+), 72 deletions(-) diff --git a/PENDING.md b/PENDING.md index 66347603aabf..7db3b567db4f 100644 --- a/PENDING.md +++ b/PENDING.md @@ -58,5 +58,6 @@ BUG FIXES * Gaia * SDK + * \#2967 Change ordering of `mint.BeginBlocker` and `distr.BeginBlocker`, recalculate inflation each block * Tendermint diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index e88e4c7c3c5c..f1fe47c09843 100644 --- a/cmd/gaia/app/app.go +++ b/cmd/gaia/app/app.go @@ -189,12 +189,12 @@ func MakeCodec() *codec.Codec { // application updates every end block func (app *GaiaApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { - // distribute rewards from previous block - distr.BeginBlocker(ctx, req, app.distrKeeper) - - // mint new tokens for this new block + // mint new tokens for the previous block mint.BeginBlocker(ctx, app.mintKeeper) + // distribute rewards for the previous block + distr.BeginBlocker(ctx, req, app.distrKeeper) + // slash anyone who double signed. // NOTE: This should happen after distr.BeginBlocker so that // there is nothing left over in the validator fee pool, diff --git a/cmd/gaia/app/export.go b/cmd/gaia/app/export.go index 041f4560e486..346830f90c9f 100644 --- a/cmd/gaia/app/export.go +++ b/cmd/gaia/app/export.go @@ -56,34 +56,13 @@ func (app *GaiaApp) ExportAppStateAndValidators(forZeroHeight bool) ( // prepare for fresh start at zero height func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context) { - /* TODO XXX check some invariants */ - - height := ctx.BlockHeight() - - valAccum := sdk.ZeroDec() - vdiIter := func(_ int64, vdi distr.ValidatorDistInfo) bool { - lastValPower := app.stakeKeeper.GetLastValidatorPower(ctx, vdi.OperatorAddr) - valAccum = valAccum.Add(vdi.GetValAccum(height, sdk.NewDecFromInt(lastValPower))) - return false - } - app.distrKeeper.IterateValidatorDistInfos(ctx, vdiIter) - - lastTotalPower := sdk.NewDecFromInt(app.stakeKeeper.GetLastTotalPower(ctx)) - totalAccum := app.distrKeeper.GetFeePool(ctx).GetTotalValAccum(height, lastTotalPower) - - if !totalAccum.Equal(valAccum) { - panic(fmt.Errorf("validator accum invariance: \n\tfee pool totalAccum: %v"+ - "\n\tvalidator accum \t%v\n", totalAccum.String(), valAccum.String())) - } - - fmt.Printf("accum invariant ok!\n") - - /* END TODO XXX */ + /* Just to be safe, assert the invariants on current state. */ + app.assertRuntimeInvariantsOnContext(ctx) /* Handle fee distribution state. */ // withdraw all delegator & validator rewards - vdiIter = func(_ int64, valInfo distr.ValidatorDistInfo) (stop bool) { + vdiIter := func(_ int64, valInfo distr.ValidatorDistInfo) (stop bool) { err := app.distrKeeper.WithdrawValidatorRewardsAll(ctx, valInfo.OperatorAddr) if err != nil { panic(err) @@ -102,10 +81,19 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context) { } app.distrKeeper.IterateDelegationDistInfos(ctx, ddiIter) - // delete all distribution infos - // these will be recreated in InitGenesis - app.distrKeeper.RemoveValidatorDistInfos(ctx) - app.distrKeeper.RemoveDelegationDistInfos(ctx) + app.assertRuntimeInvariantsOnContext(ctx) + + // set distribution info withdrawal heights to 0 + app.distrKeeper.IterateDelegationDistInfos(ctx, func(_ int64, delInfo distr.DelegationDistInfo) (stop bool) { + delInfo.DelPoolWithdrawalHeight = 0 + app.distrKeeper.SetDelegationDistInfo(ctx, delInfo) + return false + }) + app.distrKeeper.IterateValidatorDistInfos(ctx, func(_ int64, valInfo distr.ValidatorDistInfo) (stop bool) { + valInfo.FeePoolWithdrawalHeight = 0 + app.distrKeeper.SetValidatorDistInfo(ctx, valInfo) + return false + }) // assert that the fee pool is empty feePool := app.distrKeeper.GetFeePool(ctx) @@ -119,7 +107,7 @@ func (app *GaiaApp) prepForZeroHeightGenesis(ctx sdk.Context) { } // reset fee pool height, save fee pool - feePool.TotalValAccum.UpdateHeight = 0 + feePool.TotalValAccum = distr.NewTotalAccum(0) app.distrKeeper.SetFeePool(ctx, feePool) /* Handle stake state. */ diff --git a/cmd/gaia/app/invariants.go b/cmd/gaia/app/invariants.go index e67e5a16d2c5..632769e2b906 100644 --- a/cmd/gaia/app/invariants.go +++ b/cmd/gaia/app/invariants.go @@ -4,6 +4,7 @@ import ( "fmt" "time" + sdk "github.com/cosmos/cosmos-sdk/types" banksim "github.com/cosmos/cosmos-sdk/x/bank/simulation" distrsim "github.com/cosmos/cosmos-sdk/x/distribution/simulation" "github.com/cosmos/cosmos-sdk/x/mock/simulation" @@ -22,9 +23,13 @@ func (app *GaiaApp) runtimeInvariants() []simulation.Invariant { } func (app *GaiaApp) assertRuntimeInvariants() { - invariants := app.runtimeInvariants() - start := time.Now() ctx := app.NewContext(false, abci.Header{Height: app.LastBlockHeight() + 1}) + app.assertRuntimeInvariantsOnContext(ctx) +} + +func (app *GaiaApp) assertRuntimeInvariantsOnContext(ctx sdk.Context) { + start := time.Now() + invariants := app.runtimeInvariants() for _, inv := range invariants { if err := inv(ctx); err != nil { panic(fmt.Errorf("invariant broken: %s", err)) diff --git a/docs/spec/mint/begin_block.md b/docs/spec/mint/begin_block.md index 9207141472cb..09182b88a576 100644 --- a/docs/spec/mint/begin_block.md +++ b/docs/spec/mint/begin_block.md @@ -1,12 +1,12 @@ # Begin-Block -Inflation occurs at the beginning of each block, however minting parameters -are only calculated once per hour. +Minting parameters are recalculated and inflation +paid at the beginning of each block. ## NextInflationRate -The target annual inflation rate is recalculated at the first block of each new -hour. The inflation is also subject to a rate change (positive or negative) +The target annual inflation rate is recalculated each block. +The inflation is also subject to a rate change (positive or negative) depending on the distance from the desired ratio (67%). The maximum rate change possible is defined to be 13% per year, however the annual inflation is capped as between 7% and 20%. @@ -14,7 +14,7 @@ as between 7% and 20%. ``` NextInflationRate(params Params, bondedRatio sdk.Dec) (inflation sdk.Dec) { inflationRateChangePerYear = (1 - bondedRatio/params.GoalBonded) * params.InflationRateChange - inflationRateChange = inflationRateChangePerYear/hrsPerYr + inflationRateChange = inflationRateChangePerYear/blocksPerYr // increase the new annual inflation for this next cycle inflation += inflationRateChange diff --git a/docs/spec/mint/state.md b/docs/spec/mint/state.md index c3133296ea5c..42a8df5182e2 100644 --- a/docs/spec/mint/state.md +++ b/docs/spec/mint/state.md @@ -8,7 +8,6 @@ The minter is a space for holding current inflation information. ```golang type Minter struct { - LastUpdate time.Time // time which the last update was made to the minter Inflation sdk.Dec // current annual inflation rate AnnualProvisions sdk.Dec // current annual exptected provisions } @@ -30,4 +29,3 @@ type Params struct { BlocksPerYear uint64 // expected blocks per year } ``` - diff --git a/x/distribution/alias.go b/x/distribution/alias.go index 31e1ed30d3cc..34813abd6df4 100644 --- a/x/distribution/alias.go +++ b/x/distribution/alias.go @@ -60,6 +60,8 @@ var ( NewMsgWithdrawValidatorRewardsAll = types.NewMsgWithdrawValidatorRewardsAll NewDecCoins = types.NewDecCoins + + NewTotalAccum = types.NewTotalAccum ) const ( diff --git a/x/distribution/types/dec_coin.go b/x/distribution/types/dec_coin.go index 221cc9c17b3b..8a9e9f0d0be1 100644 --- a/x/distribution/types/dec_coin.go +++ b/x/distribution/types/dec_coin.go @@ -193,3 +193,13 @@ func (coins DecCoins) HasNegative() bool { } return false } + +// return whether all coins are zero +func (coins DecCoins) IsZero() bool { + for _, coin := range coins { + if !coin.Amount.IsZero() { + return false + } + } + return true +} diff --git a/x/mint/abci_app.go b/x/mint/abci_app.go index 963039961a6b..58b5fa4cd5fd 100644 --- a/x/mint/abci_app.go +++ b/x/mint/abci_app.go @@ -1,31 +1,26 @@ package mint import ( - "time" - sdk "github.com/cosmos/cosmos-sdk/types" ) // Inflate every block, update inflation parameters once per hour func BeginBlocker(ctx sdk.Context, k Keeper) { - blockTime := ctx.BlockHeader().Time + // fetch stored minter & params minter := k.GetMinter(ctx) params := k.GetParams(ctx) - mintedCoin := minter.BlockProvision(params) - k.fck.AddCollectedFees(ctx, sdk.Coins{mintedCoin}) - k.sk.InflateSupply(ctx, sdk.NewDecFromInt(mintedCoin.Amount)) - - if blockTime.Sub(minter.LastUpdate) < time.Hour { - return - } - - // adjust the inflation, hourly-provision rate every hour + // recalculate inflation rate totalSupply := k.sk.TotalPower(ctx) bondedRatio := k.sk.BondedRatio(ctx) minter.Inflation = minter.NextInflationRate(params, bondedRatio) minter.AnnualProvisions = minter.NextAnnualProvisions(params, totalSupply) - minter.LastUpdate = blockTime k.SetMinter(ctx, minter) + + // mint coins, add to collected fees, update supply + mintedCoin := minter.BlockProvision(params) + k.fck.AddCollectedFees(ctx, sdk.Coins{mintedCoin}) + k.sk.InflateSupply(ctx, sdk.NewDecFromInt(mintedCoin.Amount)) + } diff --git a/x/mint/minter.go b/x/mint/minter.go index 2edf1dabea38..8bf49f99ab78 100644 --- a/x/mint/minter.go +++ b/x/mint/minter.go @@ -2,24 +2,20 @@ package mint import ( "fmt" - "time" sdk "github.com/cosmos/cosmos-sdk/types" ) // Minter represents the minting state type Minter struct { - LastUpdate time.Time `json:"last_update"` // time which the last update was made to the minter - Inflation sdk.Dec `json:"inflation"` // current annual inflation rate - AnnualProvisions sdk.Dec `json:"annual_provisions"` // current annual expected provisions + Inflation sdk.Dec `json:"inflation"` // current annual inflation rate + AnnualProvisions sdk.Dec `json:"annual_provisions"` // current annual expected provisions } // Create a new minter object -func NewMinter(lastUpdate time.Time, inflation, - annualProvisions sdk.Dec) Minter { +func NewMinter(inflation, annualProvisions sdk.Dec) Minter { return Minter{ - LastUpdate: lastUpdate, Inflation: inflation, AnnualProvisions: annualProvisions, } @@ -28,7 +24,6 @@ func NewMinter(lastUpdate time.Time, inflation, // minter object for a new chain func InitialMinter(inflation sdk.Dec) Minter { return NewMinter( - time.Unix(0, 0), inflation, sdk.NewDec(0), ) @@ -50,8 +45,6 @@ func validateMinter(minter Minter) error { return nil } -var hrsPerYr = sdk.NewDec(8766) // as defined by a julian year of 365.25 days - // get the new inflation rate for the next hour func (m Minter) NextInflationRate(params Params, bondedRatio sdk.Dec) ( inflation sdk.Dec) { @@ -66,7 +59,7 @@ func (m Minter) NextInflationRate(params Params, bondedRatio sdk.Dec) ( inflationRateChangePerYear := sdk.OneDec(). Sub(bondedRatio.Quo(params.GoalBonded)). Mul(params.InflationRateChange) - inflationRateChange := inflationRateChangePerYear.Quo(hrsPerYr) + inflationRateChange := inflationRateChangePerYear.Quo(sdk.NewDec(int64(params.BlocksPerYear))) // increase the new annual inflation for this next cycle inflation = m.Inflation.Add(inflationRateChange) diff --git a/x/mint/minter_test.go b/x/mint/minter_test.go index a393b3a048cb..18540862899d 100644 --- a/x/mint/minter_test.go +++ b/x/mint/minter_test.go @@ -12,6 +12,7 @@ import ( func TestNextInflation(t *testing.T) { minter := DefaultInitialMinter() params := DefaultParams() + blocksPerYr := sdk.NewDec(int64(params.BlocksPerYear)) // Governing Mechanism: // inflationRateChangePerYear = (1- BondedRatio/ GoalBonded) * MaxInflationRateChange @@ -20,24 +21,24 @@ func TestNextInflation(t *testing.T) { bondedRatio, setInflation, expChange sdk.Dec }{ // with 0% bonded atom supply the inflation should increase by InflationRateChange - {sdk.ZeroDec(), sdk.NewDecWithPrec(7, 2), params.InflationRateChange.Quo(hrsPerYr)}, + {sdk.ZeroDec(), sdk.NewDecWithPrec(7, 2), params.InflationRateChange.Quo(blocksPerYr)}, // 100% bonded, starting at 20% inflation and being reduced // (1 - (1/0.67))*(0.13/8667) {sdk.OneDec(), sdk.NewDecWithPrec(20, 2), - sdk.OneDec().Sub(sdk.OneDec().Quo(params.GoalBonded)).Mul(params.InflationRateChange).Quo(hrsPerYr)}, + sdk.OneDec().Sub(sdk.OneDec().Quo(params.GoalBonded)).Mul(params.InflationRateChange).Quo(blocksPerYr)}, // 50% bonded, starting at 10% inflation and being increased {sdk.NewDecWithPrec(5, 1), sdk.NewDecWithPrec(10, 2), - sdk.OneDec().Sub(sdk.NewDecWithPrec(5, 1).Quo(params.GoalBonded)).Mul(params.InflationRateChange).Quo(hrsPerYr)}, + sdk.OneDec().Sub(sdk.NewDecWithPrec(5, 1).Quo(params.GoalBonded)).Mul(params.InflationRateChange).Quo(blocksPerYr)}, // test 7% minimum stop (testing with 100% bonded) {sdk.OneDec(), sdk.NewDecWithPrec(7, 2), sdk.ZeroDec()}, - {sdk.OneDec(), sdk.NewDecWithPrec(70001, 6), sdk.NewDecWithPrec(-1, 6)}, + {sdk.OneDec(), sdk.NewDecWithPrec(700000001, 10), sdk.NewDecWithPrec(-1, 10)}, // test 20% maximum stop (testing with 0% bonded) {sdk.ZeroDec(), sdk.NewDecWithPrec(20, 2), sdk.ZeroDec()}, - {sdk.ZeroDec(), sdk.NewDecWithPrec(199999, 6), sdk.NewDecWithPrec(1, 6)}, + {sdk.ZeroDec(), sdk.NewDecWithPrec(1999999999, 10), sdk.NewDecWithPrec(1, 10)}, // perfect balance shouldn't change inflation {sdk.NewDecWithPrec(67, 2), sdk.NewDecWithPrec(15, 2), sdk.ZeroDec()}, @@ -95,8 +96,36 @@ func BenchmarkBlockProvision(b *testing.B) { r1 := rand.New(s1) minter.AnnualProvisions = sdk.NewDec(r1.Int63n(1000000)) - // run the Fib function b.N times + // run the BlockProvision function b.N times for n := 0; n < b.N; n++ { minter.BlockProvision(params) } } + +// Next inflation benchmarking +// BenchmarkNextInflation-4 1000000 1828 ns/op +func BenchmarkNextInflation(b *testing.B) { + minter := InitialMinter(sdk.NewDecWithPrec(1, 1)) + params := DefaultParams() + bondedRatio := sdk.NewDecWithPrec(1, 1) + + // run the NextInflationRate function b.N times + for n := 0; n < b.N; n++ { + minter.NextInflationRate(params, bondedRatio) + } + +} + +// Next annual provisions benchmarking +// BenchmarkNextAnnualProvisions-4 5000000 251 ns/op +func BenchmarkNextAnnualProvisions(b *testing.B) { + minter := InitialMinter(sdk.NewDecWithPrec(1, 1)) + params := DefaultParams() + totalSupply := sdk.NewDec(100000000000000) + + // run the NextAnnualProvisions function b.N times + for n := 0; n < b.N; n++ { + minter.NextAnnualProvisions(params, totalSupply) + } + +}