From 17fcad3abfbdf042f5409d9e498e8589c63c52ea Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 9 Dec 2019 18:10:33 -0800 Subject: [PATCH 01/19] add history module and some tests --- x/staking/abci.go | 31 +++++++++++++++ x/staking/keeper/historical_info.go | 35 +++++++++++++++++ x/staking/keeper/historical_info_test.go | 34 ++++++++++++++++ x/staking/keeper/params.go | 8 ++++ x/staking/keeper/querier.go | 11 ++++++ x/staking/module.go | 4 +- x/staking/simulation/genesis.go | 2 +- x/staking/types/errors.go | 21 ++++++---- x/staking/types/historical_info.go | 49 ++++++++++++++++++++++++ x/staking/types/keys.go | 10 +++++ x/staking/types/params.go | 43 ++++++++++++--------- x/staking/types/querier.go | 11 ++++++ x/staking/types/validator.go | 23 +++++++++++ x/staking/types/validator_test.go | 32 ++++++++++++++++ 14 files changed, 286 insertions(+), 28 deletions(-) create mode 100644 x/staking/abci.go create mode 100644 x/staking/keeper/historical_info.go create mode 100644 x/staking/keeper/historical_info_test.go create mode 100644 x/staking/types/historical_info.go diff --git a/x/staking/abci.go b/x/staking/abci.go new file mode 100644 index 000000000000..3a87dd3e545f --- /dev/null +++ b/x/staking/abci.go @@ -0,0 +1,31 @@ +package staking + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +// BeginBlocker will +func BeginBlocker(ctx sdk.Context, k Keeper) { + entryNum := k.HistoricalEntries(ctx) + // if there is no need to persist historicalInfo, return + if entryNum == 0 { + return + } + + // Create HistoricalInfo struct + lastVals := k.GetLastValidators(ctx) + types.Validators(lastVals).Sort() + historicalEntry := types.HistoricalInfo{ + Header: ctx.BlockHeader(), + ValSet: lastVals, + } + + // Set latest HistoricalInfo at current height + k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) + + // prune store to ensure we only have parameter-defined historical entries + if ctx.BlockHeight() > int64(entryNum) { + k.DeleteHistoricalInfo(ctx, ctx.BlockHeight()-int64(entryNum)) + } +} diff --git a/x/staking/keeper/historical_info.go b/x/staking/keeper/historical_info.go new file mode 100644 index 000000000000..c00f81b42583 --- /dev/null +++ b/x/staking/keeper/historical_info.go @@ -0,0 +1,35 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/types" +) + +func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (hi types.HistoricalInfo, found bool) { + store := ctx.KVStore(k.storeKey) + key := types.GetHistoricalInfoKey(height) + + value := store.Get(key) + + if value == nil { + return types.HistoricalInfo{}, false + } + + hi = types.MustUnmarshalHistoricalInfo(k.cdc, value) + return hi, true +} + +func (k Keeper) SetHistoricalInfo(ctx sdk.Context, height int64, hi types.HistoricalInfo) { + store := ctx.KVStore(k.storeKey) + key := types.GetHistoricalInfoKey(height) + + value := types.MustMarshalHistoricalInfo(k.cdc, hi) + store.Set(key, value) +} + +func (k Keeper) DeleteHistoricalInfo(ctx sdk.Context, height int64) { + store := ctx.KVStore(k.storeKey) + key := types.GetHistoricalInfoKey(height) + + store.Delete(key) +} diff --git a/x/staking/keeper/historical_info_test.go b/x/staking/keeper/historical_info_test.go new file mode 100644 index 000000000000..fb77ae952e76 --- /dev/null +++ b/x/staking/keeper/historical_info_test.go @@ -0,0 +1,34 @@ +package keeper + +import ( + "sort" + "testing" + + "github.com/cosmos/cosmos-sdk/x/staking/types" + + "github.com/stretchr/testify/require" +) + +func TestHistoricalInfo(t *testing.T) { + ctx, _, keeper, _ := CreateTestInput(t, false, 10) + var validators []types.Validator + + for i, valAddr := range addrVals { + validators = append(validators, types.NewValidator(valAddr, PKs[i], types.Description{})) + } + + hi := types.NewHistoricalInfo(ctx.BlockHeader(), validators) + + keeper.SetHistoricalInfo(ctx, 2, hi) + + recv, found := keeper.GetHistoricalInfo(ctx, 2) + require.True(t, found, "HistoricalInfo not found after set") + require.Equal(t, hi, recv, "HistoricalInfo not equal") + require.True(t, sort.IsSorted(types.Validators(recv.ValSet)), "HistoricalInfo validators is not sorted") + + keeper.DeleteHistoricalInfo(ctx, 2) + + recv, found = keeper.GetHistoricalInfo(ctx, 2) + require.False(t, found, "HistoricalInfo found after delete") + require.Equal(t, types.HistoricalInfo{}, recv, "HistoricalInfo is not empty") +} diff --git a/x/staking/keeper/params.go b/x/staking/keeper/params.go index dede56b2c19d..11f4b57bc55b 100644 --- a/x/staking/keeper/params.go +++ b/x/staking/keeper/params.go @@ -37,6 +37,13 @@ func (k Keeper) MaxEntries(ctx sdk.Context) (res uint16) { return } +// HistoricalEntries = number of historical info entries +// to persist in store +func (k Keeper) HistoricalEntries(ctx sdk.Context) (res uint16) { + k.paramstore.Get(ctx, types.KeyHistoricalEntries, &res) + return +} + // BondDenom - Bondable coin denomination func (k Keeper) BondDenom(ctx sdk.Context) (res string) { k.paramstore.Get(ctx, types.KeyBondDenom, &res) @@ -49,6 +56,7 @@ func (k Keeper) GetParams(ctx sdk.Context) types.Params { k.UnbondingTime(ctx), k.MaxValidators(ctx), k.MaxEntries(ctx), + k.HistoricalEntries(ctx), k.BondDenom(ctx), ) } diff --git a/x/staking/keeper/querier.go b/x/staking/keeper/querier.go index d97bb07b10ef..5271bef10cf1 100644 --- a/x/staking/keeper/querier.go +++ b/x/staking/keeper/querier.go @@ -327,6 +327,17 @@ func queryRedelegations(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byt return res, nil } +func queryHistoricalInfo(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]byte, sdk.Error) { + var params types.QueryHistoricalInfoParams + + err := types.ModuleCdc.UnmarshalJSON(req.Data, ¶ms) + if err != nil { + return nil, sdk.ErrUnknownRequest(string(req.Data)) + } + return nil, nil + +} + func queryPool(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { bondDenom := k.BondDenom(ctx) bondedPool := k.GetBondedPool(ctx) diff --git a/x/staking/module.go b/x/staking/module.go index 45a06e67244a..69a8cb940504 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -165,7 +165,9 @@ func (am AppModule) ExportGenesis(ctx sdk.Context) json.RawMessage { } // BeginBlock returns the begin blocker for the staking module. -func (AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} +func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { + BeginBlocker(ctx, am.keeper) +} // EndBlock returns the end blocker for the staking module. It returns no validator // updates. diff --git a/x/staking/simulation/genesis.go b/x/staking/simulation/genesis.go index d0455386298c..cca02bee8cc9 100644 --- a/x/staking/simulation/genesis.go +++ b/x/staking/simulation/genesis.go @@ -50,7 +50,7 @@ func RandomizedGenState(simState *module.SimulationState) { // NewSimulationManager constructor for this to work simState.UnbondTime = unbondTime - params := types.NewParams(simState.UnbondTime, maxValidators, 7, sdk.DefaultBondDenom) + params := types.NewParams(simState.UnbondTime, maxValidators, 7, 3, sdk.DefaultBondDenom) // validators & delegations var ( diff --git a/x/staking/types/errors.go b/x/staking/types/errors.go index 012f9421f8ee..1412977c1924 100644 --- a/x/staking/types/errors.go +++ b/x/staking/types/errors.go @@ -14,14 +14,15 @@ type CodeType = sdk.CodeType const ( DefaultCodespace sdk.CodespaceType = ModuleName - CodeInvalidValidator CodeType = 101 - CodeInvalidDelegation CodeType = 102 - CodeInvalidInput CodeType = 103 - CodeValidatorJailed CodeType = 104 - CodeInvalidAddress CodeType = sdk.CodeInvalidAddress - CodeUnauthorized CodeType = sdk.CodeUnauthorized - CodeInternal CodeType = sdk.CodeInternal - CodeUnknownRequest CodeType = sdk.CodeUnknownRequest + CodeInvalidValidator CodeType = 101 + CodeInvalidDelegation CodeType = 102 + CodeInvalidInput CodeType = 103 + CodeValidatorJailed CodeType = 104 + CodeInvalidHistoricalInfo CodeType = 105 + CodeInvalidAddress CodeType = sdk.CodeInvalidAddress + CodeUnauthorized CodeType = sdk.CodeUnauthorized + CodeInternal CodeType = sdk.CodeInternal + CodeUnknownRequest CodeType = sdk.CodeUnknownRequest ) //validator @@ -212,3 +213,7 @@ func ErrNeitherShareMsgsGiven(codespace sdk.CodespaceType) sdk.Error { func ErrMissingSignature(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidValidator, "missing signature") } + +func ErrInvalidHistoricalInfo(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidHistoricalInfo, "invalid historical info") +} diff --git a/x/staking/types/historical_info.go b/x/staking/types/historical_info.go new file mode 100644 index 000000000000..f93268b6cade --- /dev/null +++ b/x/staking/types/historical_info.go @@ -0,0 +1,49 @@ +package types + +import ( + "sort" + + abci "github.com/tendermint/tendermint/abci/types" + + "github.com/cosmos/cosmos-sdk/codec" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +type HistoricalInfo struct { + Header abci.Header + ValSet []Validator +} + +func NewHistoricalInfo(header abci.Header, valSet []Validator) HistoricalInfo { + return HistoricalInfo{ + Header: header, + ValSet: valSet, + } +} + +func MustMarshalHistoricalInfo(cdc *codec.Codec, hi HistoricalInfo) []byte { + return cdc.MustMarshalBinaryLengthPrefixed(hi) +} + +func MustUnmarshalHistoricalInfo(cdc *codec.Codec, value []byte) HistoricalInfo { + hi, err := UnmarshalHistoricalInfo(cdc, value) + if err != nil { + panic(err) + } + return hi +} + +func UnmarshalHistoricalInfo(cdc *codec.Codec, value []byte) (hi HistoricalInfo, err error) { + err = cdc.UnmarshalBinaryLengthPrefixed(value, &hi) + return hi, err +} + +func ValidateHistoricalInfo(hi HistoricalInfo) error { + if hi.ValSet != nil { + return sdkerrors.Wrap(ErrInvalidHistoricalInfo(DefaultCodespace), "ValidatorSer is nil") + } + if !sort.IsSorted(Validators(hi.ValSet)) { + return sdkerrors.Wrap(ErrInvalidHistoricalInfo(DefaultCodespace), "ValidatorSet is not sorted by address") + } + return nil +} diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 372b0b924294..cf3144bbb195 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -2,6 +2,7 @@ package types import ( "encoding/binary" + "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -45,6 +46,8 @@ var ( UnbondingQueueKey = []byte{0x41} // prefix for the timestamps in unbonding queue RedelegationQueueKey = []byte{0x42} // prefix for the timestamps in redelegations queue ValidatorQueueKey = []byte{0x43} // prefix for the timestamps in validator queue + + HistoricalInfoKey = []byte{0x50} // prefix for the historical info ) // gets the key for the validator with address @@ -278,3 +281,10 @@ func GetREDsByDelToValDstIndexKey(delAddr sdk.AccAddress, valDstAddr sdk.ValAddr GetREDsToValDstIndexKey(valDstAddr), delAddr.Bytes()...) } + +//________________________________________________________________________________ + +// gets the key for historicalinfo +func GetHistoricalInfoKey(height int64) []byte { + return append(HistoricalInfoKey, []byte(fmt.Sprintf("%d", height))...) +} diff --git a/x/staking/types/params.go b/x/staking/types/params.go index 8be8cff4f24b..89e184c3195b 100644 --- a/x/staking/types/params.go +++ b/x/staking/types/params.go @@ -22,36 +22,41 @@ const ( // Default maximum entries in a UBD/RED pair DefaultMaxEntries uint16 = 7 + + DefaultHistoricalEntries uint16 = 0 ) // nolint - Keys for parameter access var ( - KeyUnbondingTime = []byte("UnbondingTime") - KeyMaxValidators = []byte("MaxValidators") - KeyMaxEntries = []byte("KeyMaxEntries") - KeyBondDenom = []byte("BondDenom") + KeyUnbondingTime = []byte("UnbondingTime") + KeyMaxValidators = []byte("MaxValidators") + KeyMaxEntries = []byte("KeyMaxEntries") + KeyBondDenom = []byte("BondDenom") + KeyHistoricalEntries = []byte("HistoricalEntries") ) var _ params.ParamSet = (*Params)(nil) // Params defines the high level settings for staking type Params struct { - UnbondingTime time.Duration `json:"unbonding_time" yaml:"unbonding_time"` // time duration of unbonding - MaxValidators uint16 `json:"max_validators" yaml:"max_validators"` // maximum number of validators (max uint16 = 65535) - MaxEntries uint16 `json:"max_entries" yaml:"max_entries"` // max entries for either unbonding delegation or redelegation (per pair/trio) + UnbondingTime time.Duration `json:"unbonding_time" yaml:"unbonding_time"` // time duration of unbonding + MaxValidators uint16 `json:"max_validators" yaml:"max_validators"` // maximum number of validators (max uint16 = 65535) + MaxEntries uint16 `json:"max_entries" yaml:"max_entries"` // max entries for either unbonding delegation or redelegation (per pair/trio) + HistoricalEntries uint16 `json:"historical_entries" yaml:"historical_entries"` // number of historical entries to persist // note: we need to be a bit careful about potential overflow here, since this is user-determined BondDenom string `json:"bond_denom" yaml:"bond_denom"` // bondable coin denomination } // NewParams creates a new Params instance -func NewParams(unbondingTime time.Duration, maxValidators, maxEntries uint16, +func NewParams(unbondingTime time.Duration, maxValidators, maxEntries, historicalEntries uint16, bondDenom string) Params { return Params{ - UnbondingTime: unbondingTime, - MaxValidators: maxValidators, - MaxEntries: maxEntries, - BondDenom: bondDenom, + UnbondingTime: unbondingTime, + MaxValidators: maxValidators, + MaxEntries: maxEntries, + HistoricalEntries: historicalEntries, + BondDenom: bondDenom, } } @@ -61,6 +66,7 @@ func (p *Params) ParamSetPairs() params.ParamSetPairs { {Key: KeyUnbondingTime, Value: &p.UnbondingTime}, {Key: KeyMaxValidators, Value: &p.MaxValidators}, {Key: KeyMaxEntries, Value: &p.MaxEntries}, + {Key: KeyHistoricalEntries, Value: &p.HistoricalEntries}, {Key: KeyBondDenom, Value: &p.BondDenom}, } } @@ -75,17 +81,18 @@ func (p Params) Equal(p2 Params) bool { // DefaultParams returns a default set of parameters. func DefaultParams() Params { - return NewParams(DefaultUnbondingTime, DefaultMaxValidators, DefaultMaxEntries, sdk.DefaultBondDenom) + return NewParams(DefaultUnbondingTime, DefaultMaxValidators, DefaultMaxEntries, DefaultHistoricalEntries, sdk.DefaultBondDenom) } // String returns a human readable string representation of the parameters. func (p Params) String() string { return fmt.Sprintf(`Params: - Unbonding Time: %s - Max Validators: %d - Max Entries: %d - Bonded Coin Denom: %s`, p.UnbondingTime, - p.MaxValidators, p.MaxEntries, p.BondDenom) + Unbonding Time: %s + Max Validators: %d + Max Entries: %d + Historical Entries: %d + Bonded Coin Denom: %s`, p.UnbondingTime, + p.MaxValidators, p.MaxEntries, p.HistoricalEntries, p.BondDenom) } // unmarshal the current staking params value from store key or panic diff --git a/x/staking/types/querier.go b/x/staking/types/querier.go index ba38a55742f5..671d215760a0 100644 --- a/x/staking/types/querier.go +++ b/x/staking/types/querier.go @@ -20,6 +20,7 @@ const ( QueryDelegatorValidator = "delegatorValidator" QueryPool = "pool" QueryParameters = "parameters" + QueryHistoricalInfo = "historicalInfo" ) // defines the params for the following queries: @@ -96,3 +97,13 @@ type QueryValidatorsParams struct { func NewQueryValidatorsParams(page, limit int, status string) QueryValidatorsParams { return QueryValidatorsParams{page, limit, status} } + +// QueryHistoricalInfoParams defines the params for the following queries: +// - 'custom/staking/historicalInfo' +type QueryHistoricalInfoParams struct { + Height int +} + +func NewQueryHistoricalInfoParams(height int) QueryHistoricalInfoParams { + return QueryHistoricalInfoParams{height} +} diff --git a/x/staking/types/validator.go b/x/staking/types/validator.go index df68c177e930..37e6cbc02575 100644 --- a/x/staking/types/validator.go +++ b/x/staking/types/validator.go @@ -3,6 +3,7 @@ package types import ( "bytes" "fmt" + "sort" "strings" "time" @@ -102,6 +103,28 @@ func (v Validators) ToSDKValidators() (validators []exported.ValidatorI) { return validators } +// Sort Validators sorts validator array in ascending operator address order +func (v Validators) Sort() { + sort.Sort(v) +} + +// Implements sort interface +func (v Validators) Len() int { + return len(v) +} + +// Implements sort interface +func (v Validators) Less(i, j int) bool { + return bytes.Compare(v[i].OperatorAddress, v[j].OperatorAddress) == -1 +} + +// Implements sort interface +func (v Validators) Swap(i, j int) { + it := v[i] + v[i] = v[j] + v[j] = it +} + // NewValidator - initialize a new validator func NewValidator(operator sdk.ValAddress, pubKey crypto.PubKey, description Description) Validator { return Validator{ diff --git a/x/staking/types/validator_test.go b/x/staking/types/validator_test.go index 6f8243efab4b..61ad276ad005 100644 --- a/x/staking/types/validator_test.go +++ b/x/staking/types/validator_test.go @@ -2,10 +2,14 @@ package types import ( "fmt" + "math/rand" + "reflect" + "sort" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto/ed25519" tmtypes "github.com/tendermint/tendermint/types" yaml "gopkg.in/yaml.v2" @@ -303,3 +307,31 @@ func TestValidatorMarshalYAML(t *testing.T) { `, validator.OperatorAddress.String(), bechifiedPub) require.Equal(t, want, string(bs)) } + +// Check that sort will create deterministic ordering of validators +func TestValidatorsSortDeterminism(t *testing.T) { + vals := make([]Validator, 10) + sortedVals := make([]Validator, 10) + + // Create random validator slice + for i := range vals { + pk := ed25519.GenPrivKey().PubKey() + vals[i] = NewValidator(sdk.ValAddress(pk.Address()), pk, Description{}) + } + + // Save sorted copy + sort.Sort(Validators(vals)) + copy(sortedVals, vals) + + // Randomly shuffle validators, sort, and check it is equal to original sort + for i := 0; i < 10; i++ { + rand.Shuffle(10, func(i, j int) { + it := vals[i] + vals[i] = vals[j] + vals[j] = it + }) + + sort.Sort(Validators(vals)) + require.True(t, reflect.DeepEqual(sortedVals, vals), "Validator sort returned different slices") + } +} From 2b052b414c99c31b38240905c7e6370da4146adb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 10 Dec 2019 13:27:57 -0800 Subject: [PATCH 02/19] complete query and write tests --- x/staking/keeper/querier.go | 14 +++++++- x/staking/keeper/querier_test.go | 59 ++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/x/staking/keeper/querier.go b/x/staking/keeper/querier.go index 5271bef10cf1..15bc59ee5ff6 100644 --- a/x/staking/keeper/querier.go +++ b/x/staking/keeper/querier.go @@ -38,6 +38,8 @@ func NewQuerier(k Keeper) sdk.Querier { return queryDelegatorValidators(ctx, req, k) case types.QueryDelegatorValidator: return queryDelegatorValidator(ctx, req, k) + case types.QueryHistoricalInfo: + return queryHistoricalInfo(ctx, req, k) case types.QueryPool: return queryPool(ctx, k) case types.QueryParameters: @@ -334,8 +336,18 @@ func queryHistoricalInfo(ctx sdk.Context, req abci.RequestQuery, k Keeper) ([]by if err != nil { return nil, sdk.ErrUnknownRequest(string(req.Data)) } - return nil, nil + hi, found := k.GetHistoricalInfo(ctx, params.Height) + if !found { + return nil, types.ErrNoHistoricalInfo(types.DefaultCodespace) + } + + res, err := codec.MarshalJSONIndent(types.ModuleCdc, hi) + if err != nil { + return nil, sdk.ErrInternal(sdk.AppendMsgToErr("could not marshal result to JSON", err.Error())) + } + + return res, nil } func queryPool(ctx sdk.Context, k Keeper) ([]byte, sdk.Error) { diff --git a/x/staking/keeper/querier_test.go b/x/staking/keeper/querier_test.go index 2dac9b6f99ab..b91c92a06423 100644 --- a/x/staking/keeper/querier_test.go +++ b/x/staking/keeper/querier_test.go @@ -31,6 +31,13 @@ func TestNewQuerier(t *testing.T) { keeper.SetValidatorByPowerIndex(ctx, validators[i]) } + header := abci.Header{ + ChainID: "HelloChain", + Height: 5, + } + hi := types.NewHistoricalInfo(header, validators[:]) + keeper.SetHistoricalInfo(ctx, 5, hi) + query := abci.RequestQuery{ Path: "", Data: []byte{}, @@ -86,6 +93,16 @@ func TestNewQuerier(t *testing.T) { _, err = querier(ctx, []string{"redelegations"}, query) require.Nil(t, err) + + queryHisParams := types.NewQueryHistoricalInfoParams(5) + bz, errRes = cdc.MarshalJSON(queryHisParams) + require.Nil(t, errRes) + + query.Path = "/custom/staking/historicalInfo" + query.Data = bz + + _, err = querier(ctx, []string{"historicalInfo"}, query) + require.Nil(t, err) } func TestQueryParametersPool(t *testing.T) { @@ -551,3 +568,45 @@ func TestQueryUnbondingDelegation(t *testing.T) { require.NoError(t, cdc.UnmarshalJSON(res, &ubDels)) require.Equal(t, 0, len(ubDels)) } + +func TestQueryHistoricalInfo(t *testing.T) { + cdc := codec.New() + ctx, _, keeper, _ := CreateTestInput(t, false, 10000) + + // Create Validators and Delegation + val1 := types.NewValidator(addrVal1, pk1, types.Description{}) + val2 := types.NewValidator(addrVal2, pk2, types.Description{}) + vals := []types.Validator{val1, val2} + keeper.SetValidator(ctx, val1) + keeper.SetValidator(ctx, val2) + + header := abci.Header{ + ChainID: "HelloChain", + Height: 5, + } + hi := types.NewHistoricalInfo(header, vals) + keeper.SetHistoricalInfo(ctx, 5, hi) + + queryHistoricalParams := types.NewQueryHistoricalInfoParams(4) + bz, errRes := cdc.MarshalJSON(queryHistoricalParams) + require.Nil(t, errRes) + query := abci.RequestQuery{ + Path: "/custom/staking/historicalInfo", + Data: bz, + } + res, err := queryHistoricalInfo(ctx, query, keeper) + require.NotNil(t, err, "Invalid query passed") + require.Nil(t, res, "Invalid query returned non-nil result") + + queryHistoricalParams = types.NewQueryHistoricalInfoParams(5) + bz, errRes = cdc.MarshalJSON(queryHistoricalParams) + require.Nil(t, errRes) + query.Data = bz + res, err = queryHistoricalInfo(ctx, query, keeper) + require.Nil(t, err, "Valid query passed") + require.NotNil(t, res, "Valid query returned nil result") + + var recv types.HistoricalInfo + require.NoError(t, cdc.UnmarshalJSON(res, &recv)) + require.Equal(t, hi, recv, "HistoricalInfo query returned wrong result") +} From 481ee0df3dfd9668f710dba67791c82ae8400469 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 10 Dec 2019 13:28:14 -0800 Subject: [PATCH 03/19] complete query and write tests --- x/staking/types/errors.go | 4 +++ x/staking/types/historical_info.go | 1 + x/staking/types/historical_info_test.go | 35 +++++++++++++++++++++++++ x/staking/types/querier.go | 4 +-- 4 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 x/staking/types/historical_info_test.go diff --git a/x/staking/types/errors.go b/x/staking/types/errors.go index 1412977c1924..523e720e2d98 100644 --- a/x/staking/types/errors.go +++ b/x/staking/types/errors.go @@ -217,3 +217,7 @@ func ErrMissingSignature(codespace sdk.CodespaceType) sdk.Error { func ErrInvalidHistoricalInfo(codespace sdk.CodespaceType) sdk.Error { return sdk.NewError(codespace, CodeInvalidHistoricalInfo, "invalid historical info") } + +func ErrNoHistoricalInfo(codespace sdk.CodespaceType) sdk.Error { + return sdk.NewError(codespace, CodeInvalidHistoricalInfo, "no historical info found") +} diff --git a/x/staking/types/historical_info.go b/x/staking/types/historical_info.go index f93268b6cade..3ab36eef57fc 100644 --- a/x/staking/types/historical_info.go +++ b/x/staking/types/historical_info.go @@ -15,6 +15,7 @@ type HistoricalInfo struct { } func NewHistoricalInfo(header abci.Header, valSet []Validator) HistoricalInfo { + sort.Sort(Validators(valSet)) return HistoricalInfo{ Header: header, ValSet: valSet, diff --git a/x/staking/types/historical_info_test.go b/x/staking/types/historical_info_test.go new file mode 100644 index 000000000000..e2c455cb8054 --- /dev/null +++ b/x/staking/types/historical_info_test.go @@ -0,0 +1,35 @@ +package types + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" +) + +func TestHistoricalInfo(t *testing.T) { + validators := []Validator{ + NewValidator(valAddr1, pk1, Description{}), + NewValidator(valAddr2, pk2, Description{}), + NewValidator(valAddr3, pk3, Description{}), + } + header := abci.Header{ + ChainID: "hello", + Height: 5, + } + hi := NewHistoricalInfo(header, validators) + require.True(t, sort.IsSorted(Validators(hi.ValSet)), "Validators are not sorted") + + var value []byte + require.NotPanics(t, func() { + value = MustMarshalHistoricalInfo(ModuleCdc, hi) + }) + + require.NotNil(t, value, "Marshalled HistoricalInfo is nil") + + recv, err := UnmarshalHistoricalInfo(ModuleCdc, value) + require.Nil(t, err, "Unmarshalling HistoricalInfo failed") + require.Equal(t, hi, recv, "Unmarshalled HistoricalInfo is different from original") + require.True(t, sort.IsSorted(Validators(hi.ValSet)), "Validators are not sorted") +} diff --git a/x/staking/types/querier.go b/x/staking/types/querier.go index 671d215760a0..15dfa60eb6ab 100644 --- a/x/staking/types/querier.go +++ b/x/staking/types/querier.go @@ -101,9 +101,9 @@ func NewQueryValidatorsParams(page, limit int, status string) QueryValidatorsPar // QueryHistoricalInfoParams defines the params for the following queries: // - 'custom/staking/historicalInfo' type QueryHistoricalInfoParams struct { - Height int + Height int64 } -func NewQueryHistoricalInfoParams(height int) QueryHistoricalInfoParams { +func NewQueryHistoricalInfoParams(height int64) QueryHistoricalInfoParams { return QueryHistoricalInfoParams{height} } From 954c8b1675d7e2add3d90e5cced14f9a249996bb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 10 Dec 2019 14:32:04 -0800 Subject: [PATCH 04/19] add abci test --- x/staking/abci_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 x/staking/abci_test.go diff --git a/x/staking/abci_test.go b/x/staking/abci_test.go new file mode 100644 index 000000000000..2d5d310766c5 --- /dev/null +++ b/x/staking/abci_test.go @@ -0,0 +1,70 @@ +package staking + +import ( + "sort" + "testing" + + "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/keeper" + "github.com/cosmos/cosmos-sdk/x/staking/types" + abci "github.com/tendermint/tendermint/abci/types" +) + +func TestBeginBlocker(t *testing.T) { + ctx, _, k, _ := keeper.CreateTestInput(t, false, 10) + + // set historical entries in params to 5 + params := types.DefaultParams() + params.HistoricalEntries = 5 + k.SetParams(ctx, params) + + // set historical info at 5 which should be pruned + header := abci.Header{ + ChainID: "HelloChain", + Height: 5, + } + valSet := []Validator{ + types.NewValidator(sdk.ValAddress(keeper.Addrs[0]), keeper.PKs[0], types.Description{}), + types.NewValidator(sdk.ValAddress(keeper.Addrs[1]), keeper.PKs[1], types.Description{}), + } + hi5 := types.NewHistoricalInfo(header, valSet) + k.SetHistoricalInfo(ctx, 5, hi5) + recv, found := k.GetHistoricalInfo(ctx, 5) + require.True(t, found) + require.Equal(t, hi5, recv) + + // Set last validators in keeper + val1 := types.NewValidator(sdk.ValAddress(keeper.Addrs[2]), keeper.PKs[2], types.Description{}) + k.SetValidator(ctx, val1) + k.SetLastValidatorPower(ctx, val1.OperatorAddress, 10) + val2 := types.NewValidator(sdk.ValAddress(keeper.Addrs[3]), keeper.PKs[3], types.Description{}) + vals := []types.Validator{val1, val2} + sort.Sort(types.Validators(vals)) + k.SetValidator(ctx, val2) + k.SetLastValidatorPower(ctx, val2.OperatorAddress, 8) + + // Set Header for BeginBlock context + header = abci.Header{ + ChainID: "HelloChain", + Height: 10, + } + ctx = ctx.WithBlockHeader(header) + + BeginBlocker(ctx, k) + + // Check HistoricalInfo at height 10 is persisted + expected := types.HistoricalInfo{ + Header: header, + ValSet: vals, + } + recv, found = k.GetHistoricalInfo(ctx, 10) + require.True(t, found, "GetHistoricalInfo failed after BeginBlock") + require.Equal(t, expected, recv, "GetHistoricalInfo returned eunexpected result") + + // Check HistoricalInfo at height 5 is pruned + recv, found = k.GetHistoricalInfo(ctx, 5) + require.False(t, found, "GetHistoricalInfo did not prune") + require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo is not empty after prune") +} From da266e8a8157531ade28dd2d070a5bca4ae8a790 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 10 Dec 2019 14:49:53 -0800 Subject: [PATCH 05/19] docs and CHANGELOG --- CHANGELOG.md | 3 +++ x/staking/abci.go | 3 ++- x/staking/keeper/historical_info.go | 3 +++ x/staking/types/historical_info.go | 7 +++++++ x/staking/types/validator_test.go | 2 +- 5 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10436fa612b2..a55a56688bfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,6 +139,9 @@ that allows for arbitrary vesting periods. * `ValidateSigCountDecorator`: Validate the number of signatures in tx based on app-parameters. * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks. * (cli) [\#5223](https://github.com/cosmos/cosmos-sdk/issues/5223) Cosmos Ledger App v2.0.0 is now supported. The changes are backwards compatible and App v1.5.x is still supported. +* (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduced ability to store historical info entries in staking keeper, allows applications to introspect specified number of past headers and validator sets +* (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduces new parameter `HistoricalEntries` which allows applications to determine how many recent historical info entries they want to persist in store. Default value is 0. + ### Improvements diff --git a/x/staking/abci.go b/x/staking/abci.go index 3a87dd3e545f..c9de78c84207 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -5,7 +5,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking/types" ) -// BeginBlocker will +// BeginBlocker will persist the current header and validator set as a historical entry +// and prune the oldest entry based on the HistoricalEntries parameter func BeginBlocker(ctx sdk.Context, k Keeper) { entryNum := k.HistoricalEntries(ctx) // if there is no need to persist historicalInfo, return diff --git a/x/staking/keeper/historical_info.go b/x/staking/keeper/historical_info.go index c00f81b42583..898e23d4ba90 100644 --- a/x/staking/keeper/historical_info.go +++ b/x/staking/keeper/historical_info.go @@ -5,6 +5,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking/types" ) +// GetHistoricalInfo gets the historical info at a given height func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (hi types.HistoricalInfo, found bool) { store := ctx.KVStore(k.storeKey) key := types.GetHistoricalInfoKey(height) @@ -19,6 +20,7 @@ func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (hi types.Histo return hi, true } +// SetHistoricalInfo sets the historical info at a given height func (k Keeper) SetHistoricalInfo(ctx sdk.Context, height int64, hi types.HistoricalInfo) { store := ctx.KVStore(k.storeKey) key := types.GetHistoricalInfoKey(height) @@ -27,6 +29,7 @@ func (k Keeper) SetHistoricalInfo(ctx sdk.Context, height int64, hi types.Histor store.Set(key, value) } +// DeleteHistoricalInfo deletes the historical info at a given height func (k Keeper) DeleteHistoricalInfo(ctx sdk.Context, height int64) { store := ctx.KVStore(k.storeKey) key := types.GetHistoricalInfoKey(height) diff --git a/x/staking/types/historical_info.go b/x/staking/types/historical_info.go index 3ab36eef57fc..e3190a40ce76 100644 --- a/x/staking/types/historical_info.go +++ b/x/staking/types/historical_info.go @@ -9,11 +9,14 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) +// HistoricalInfo contains the historical information that gets stored at each height type HistoricalInfo struct { Header abci.Header ValSet []Validator } +//NewHistoricalInfo will create a historical information struct from header and valset +// it will first sort valset before inclusion into historical info func NewHistoricalInfo(header abci.Header, valSet []Validator) HistoricalInfo { sort.Sort(Validators(valSet)) return HistoricalInfo{ @@ -22,10 +25,12 @@ func NewHistoricalInfo(header abci.Header, valSet []Validator) HistoricalInfo { } } +// MustMarshalHistoricalInfo wll marshal historical info and panic on error func MustMarshalHistoricalInfo(cdc *codec.Codec, hi HistoricalInfo) []byte { return cdc.MustMarshalBinaryLengthPrefixed(hi) } +// MustUnmarshalHistoricalInfo wll unmarshal historical info and panic on error func MustUnmarshalHistoricalInfo(cdc *codec.Codec, value []byte) HistoricalInfo { hi, err := UnmarshalHistoricalInfo(cdc, value) if err != nil { @@ -34,11 +39,13 @@ func MustUnmarshalHistoricalInfo(cdc *codec.Codec, value []byte) HistoricalInfo return hi } +// UnmarshalHistoricalInfo will unmarshal historical info and return any error func UnmarshalHistoricalInfo(cdc *codec.Codec, value []byte) (hi HistoricalInfo, err error) { err = cdc.UnmarshalBinaryLengthPrefixed(value, &hi) return hi, err } +// ValidateHistoricalInfo will ensure HistoricalInfo is not nil and sorted func ValidateHistoricalInfo(hi HistoricalInfo) error { if hi.ValSet != nil { return sdkerrors.Wrap(ErrInvalidHistoricalInfo(DefaultCodespace), "ValidatorSer is nil") diff --git a/x/staking/types/validator_test.go b/x/staking/types/validator_test.go index 61ad276ad005..f705d0d8df99 100644 --- a/x/staking/types/validator_test.go +++ b/x/staking/types/validator_test.go @@ -331,7 +331,7 @@ func TestValidatorsSortDeterminism(t *testing.T) { vals[j] = it }) - sort.Sort(Validators(vals)) + Validators(vals).Sort() require.True(t, reflect.DeepEqual(sortedVals, vals), "Validator sort returned different slices") } } From 813c6349fc989f412e9bcf883d309d9bacb193bc Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 10 Dec 2019 16:17:02 -0800 Subject: [PATCH 06/19] add client query methods and CHANGELOG --- CHANGELOG.md | 1 + x/staking/client/cli/query.go | 47 ++++++++++++++++++++++++++++++++++ x/staking/client/rest/query.go | 37 ++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a55a56688bfb..b7a91be23a0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -141,6 +141,7 @@ that allows for arbitrary vesting periods. * (cli) [\#5223](https://github.com/cosmos/cosmos-sdk/issues/5223) Cosmos Ledger App v2.0.0 is now supported. The changes are backwards compatible and App v1.5.x is still supported. * (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduced ability to store historical info entries in staking keeper, allows applications to introspect specified number of past headers and validator sets * (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduces new parameter `HistoricalEntries` which allows applications to determine how many recent historical info entries they want to persist in store. Default value is 0. +* (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduces cli commands and rest routes to query historical information at a given height ### Improvements diff --git a/x/staking/client/cli/query.go b/x/staking/client/cli/query.go index ce7ca6e55be7..c2d91d4f0b70 100644 --- a/x/staking/client/cli/query.go +++ b/x/staking/client/cli/query.go @@ -1,7 +1,9 @@ package cli import ( + "errors" "fmt" + "strconv" "strings" "github.com/spf13/cobra" @@ -35,6 +37,7 @@ func GetQueryCmd(queryRoute string, cdc *codec.Codec) *cobra.Command { GetCmdQueryValidatorDelegations(queryRoute, cdc), GetCmdQueryValidatorUnbondingDelegations(queryRoute, cdc), GetCmdQueryValidatorRedelegations(queryRoute, cdc), + GetCmdQueryHistoricalInfo(queryRoute, cdc), GetCmdQueryParams(queryRoute, cdc), GetCmdQueryPool(queryRoute, cdc))...) @@ -527,6 +530,50 @@ $ %s query staking redelegation cosmos1gghjut3ccd8ay0zduzj64hwre2fxs9ld75ru9p } } +// GetCmdQueryHistoricalInfo implements the historical info query command +func GetCmdQueryHistoricalInfo(queryRoute string, cdc *codec.Codec) *cobra.Command { + return &cobra.Command{ + Use: "historical-info [height]", + Args: cobra.ExactArgs(1), + Short: "Query historical info at given height", + Long: strings.TrimSpace( + fmt.Sprintf(`Query historical info at given height. + +Example: +$ %s query staking historical-info 5 +`, + version.ClientName, + ), + ), + RunE: func(cmd *cobra.Command, args []string) error { + cliCtx := context.NewCLIContext().WithCodec(cdc) + + height, err := strconv.ParseInt(args[0], 10, 64) + if err != nil || height < 0 { + return errors.New(fmt.Sprintf("Height argument provided must be a non-negative-integer: %v", err)) + } + + bz, err := cdc.MarshalJSON(types.QueryHistoricalInfoParams{Height: height}) + if err != nil { + return err + } + + route := fmt.Sprintf("custom/%s/%s", queryRoute, types.QueryHistoricalInfo) + res, _, err := cliCtx.QueryWithData(route, bz) + if err != nil { + return err + } + + var resp types.HistoricalInfo + if err := cdc.UnmarshalJSON(res, &resp); err != nil { + return err + } + + return cliCtx.PrintOutput(resp) + }, + } +} + // GetCmdQueryPool implements the pool query command. func GetCmdQueryPool(storeName string, cdc *codec.Codec) *cobra.Command { return &cobra.Command{ diff --git a/x/staking/client/rest/query.go b/x/staking/client/rest/query.go index e0eabd870e93..d7aae7a5ff76 100644 --- a/x/staking/client/rest/query.go +++ b/x/staking/client/rest/query.go @@ -3,6 +3,7 @@ package rest import ( "fmt" "net/http" + "strconv" "strings" "github.com/gorilla/mux" @@ -86,6 +87,12 @@ func registerQueryRoutes(cliCtx context.CLIContext, r *mux.Router) { validatorUnbondingDelegationsHandlerFn(cliCtx), ).Methods("GET") + // Get HistoricalInfo at a given height + r.HandleFunc( + "/staking/historicalinfo/{height}", + historicalInfoHandlerFn(cliCtx), + ).Methods("GET") + // Get the current state of the staking pool r.HandleFunc( "/staking/pool", @@ -313,6 +320,36 @@ func validatorUnbondingDelegationsHandlerFn(cliCtx context.CLIContext) http.Hand return queryValidator(cliCtx, "custom/staking/validatorUnbondingDelegations") } +// HTTP request handler to query historical info at a given height +func historicalInfoHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + vars := mux.Vars(r) + heightStr := vars["height"] + height, err := strconv.ParseInt(heightStr, 10, 64) + if err != nil || height < 0 { + rest.WriteErrorResponse(w, http.StatusBadRequest, fmt.Sprintf("Must provide non-negative integer for height: %v", err)) + return + } + + params := types.NewQueryHistoricalInfoParams(height) + bz, err := cliCtx.Codec.MarshalJSON(params) + if err != nil { + rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) + return + } + + route := fmt.Sprint("custom/%s/%s", types.QuerierRoute, types.QueryHistoricalInfo) + res, height, err := cliCtx.QueryWithData(route, bz) + if err != nil { + rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error()) + return + } + + cliCtx = cliCtx.WithHeight(height) + rest.PostProcessResponse(w, cliCtx, res) + } +} + // HTTP request handler to query the pool information func poolHandlerFn(cliCtx context.CLIContext) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { From aa34dc77a387a16831b850ec455d7d91be12dd45 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 11 Dec 2019 12:28:13 -0800 Subject: [PATCH 07/19] implement edge case for when historicalentries param gets reduced --- x/staking/abci.go | 15 +++++++++++++-- x/staking/abci_test.go | 29 +++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/x/staking/abci.go b/x/staking/abci.go index c9de78c84207..b90d132541cc 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -26,7 +26,18 @@ func BeginBlocker(ctx sdk.Context, k Keeper) { k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) // prune store to ensure we only have parameter-defined historical entries - if ctx.BlockHeight() > int64(entryNum) { - k.DeleteHistoricalInfo(ctx, ctx.BlockHeight()-int64(entryNum)) + // in most cases, this will invole removing a single historical entry + // In the rare scenario when the historical entries gets reduced to a lower value k' + // from the original value k. k - k' entries must be deleted from the store + // Since the entries to be deleted are always in a continuous range, we can iterate + // over the historical entries starting from the most recent version to be pruned + // and then return at the first empty entry + for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- { + _, found := k.GetHistoricalInfo(ctx, i) + if found { + k.DeleteHistoricalInfo(ctx, i) + } else { + return + } } } diff --git a/x/staking/abci_test.go b/x/staking/abci_test.go index 2d5d310766c5..3bb5847be9ab 100644 --- a/x/staking/abci_test.go +++ b/x/staking/abci_test.go @@ -20,8 +20,13 @@ func TestBeginBlocker(t *testing.T) { params.HistoricalEntries = 5 k.SetParams(ctx, params) - // set historical info at 5 which should be pruned - header := abci.Header{ + // set historical info at 5, 4 which should be pruned + // and check that it has been stored + h4 := abci.Header{ + ChainID: "HelloChain", + Height: 4, + } + h5 := abci.Header{ ChainID: "HelloChain", Height: 5, } @@ -29,9 +34,14 @@ func TestBeginBlocker(t *testing.T) { types.NewValidator(sdk.ValAddress(keeper.Addrs[0]), keeper.PKs[0], types.Description{}), types.NewValidator(sdk.ValAddress(keeper.Addrs[1]), keeper.PKs[1], types.Description{}), } - hi5 := types.NewHistoricalInfo(header, valSet) + hi4 := types.NewHistoricalInfo(h4, valSet) + hi5 := types.NewHistoricalInfo(h5, valSet) + k.SetHistoricalInfo(ctx, 4, hi4) k.SetHistoricalInfo(ctx, 5, hi5) - recv, found := k.GetHistoricalInfo(ctx, 5) + recv, found := k.GetHistoricalInfo(ctx, 4) + require.True(t, found) + require.Equal(t, hi4, recv) + recv, found = k.GetHistoricalInfo(ctx, 5) require.True(t, found) require.Equal(t, hi5, recv) @@ -46,7 +56,7 @@ func TestBeginBlocker(t *testing.T) { k.SetLastValidatorPower(ctx, val2.OperatorAddress, 8) // Set Header for BeginBlock context - header = abci.Header{ + header := abci.Header{ ChainID: "HelloChain", Height: 10, } @@ -63,8 +73,11 @@ func TestBeginBlocker(t *testing.T) { require.True(t, found, "GetHistoricalInfo failed after BeginBlock") require.Equal(t, expected, recv, "GetHistoricalInfo returned eunexpected result") - // Check HistoricalInfo at height 5 is pruned + // Check HistoricalInfo at height 5, 4 is pruned + recv, found = k.GetHistoricalInfo(ctx, 4) + require.False(t, found, "GetHistoricalInfo did not prune earlier height") + require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo at height 4 is not empty after prune") recv, found = k.GetHistoricalInfo(ctx, 5) - require.False(t, found, "GetHistoricalInfo did not prune") - require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo is not empty after prune") + require.False(t, found, "GetHistoricalInfo did not prune first prune height") + require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo at height 5 is not empty after prune") } From d0e8d8880e9974a4fc63df7c5d3ec5af1697ad28 Mon Sep 17 00:00:00 2001 From: Christopher Goes Date: Wed, 11 Dec 2019 21:50:13 +0100 Subject: [PATCH 08/19] Update x/staking/abci.go --- x/staking/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/staking/abci.go b/x/staking/abci.go index b90d132541cc..24d241ab81f7 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -26,7 +26,7 @@ func BeginBlocker(ctx sdk.Context, k Keeper) { k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) // prune store to ensure we only have parameter-defined historical entries - // in most cases, this will invole removing a single historical entry + // in most cases, this will involve removing a single historical entry // In the rare scenario when the historical entries gets reduced to a lower value k' // from the original value k. k - k' entries must be deleted from the store // Since the entries to be deleted are always in a continuous range, we can iterate From 5997e1564a2de59d07c5868deb67b8a701b0e0c2 Mon Sep 17 00:00:00 2001 From: Aditya Date: Thu, 12 Dec 2019 12:26:52 -0800 Subject: [PATCH 09/19] Apply suggestions from code review Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- x/staking/types/keys.go | 2 +- x/staking/types/querier.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index cf3144bbb195..77b2a2d710f8 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -284,7 +284,7 @@ func GetREDsByDelToValDstIndexKey(delAddr sdk.AccAddress, valDstAddr sdk.ValAddr //________________________________________________________________________________ -// gets the key for historicalinfo +// GetHistoricalInfoKey gets the key for the historical info func GetHistoricalInfoKey(height int64) []byte { return append(HistoricalInfoKey, []byte(fmt.Sprintf("%d", height))...) } diff --git a/x/staking/types/querier.go b/x/staking/types/querier.go index 15dfa60eb6ab..adc387354ea9 100644 --- a/x/staking/types/querier.go +++ b/x/staking/types/querier.go @@ -104,6 +104,7 @@ type QueryHistoricalInfoParams struct { Height int64 } +// NewQueryHistoricalInfoParams creates a new QueryHistoricalInfoParams instance func NewQueryHistoricalInfoParams(height int64) QueryHistoricalInfoParams { return QueryHistoricalInfoParams{height} } From 82d386f0b72b3ce87b092af8711cb52dc88efa20 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 12 Dec 2019 14:36:13 -0800 Subject: [PATCH 10/19] apply codereview suggestions --- x/staking/abci.go | 9 +- x/staking/client/rest/query.go | 2 +- x/staking/keeper/historical_info.go | 4 +- x/staking/keeper/historical_info_test.go | 4 +- x/staking/keeper/querier_test.go | 142 +++++++++++------------ x/staking/types/historical_info.go | 10 +- x/staking/types/historical_info_test.go | 38 +++++- x/staking/types/params.go | 2 + 8 files changed, 122 insertions(+), 89 deletions(-) diff --git a/x/staking/abci.go b/x/staking/abci.go index 24d241ab81f7..16b893559421 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -16,11 +16,10 @@ func BeginBlocker(ctx sdk.Context, k Keeper) { // Create HistoricalInfo struct lastVals := k.GetLastValidators(ctx) - types.Validators(lastVals).Sort() - historicalEntry := types.HistoricalInfo{ - Header: ctx.BlockHeader(), - ValSet: lastVals, - } + historicalEntry := types.NewHistoricalInfo( + ctx.BlockHeader(), + lastVals, + ) // Set latest HistoricalInfo at current height k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) diff --git a/x/staking/client/rest/query.go b/x/staking/client/rest/query.go index 0626f1dfc089..ddd6c03c7c6b 100644 --- a/x/staking/client/rest/query.go +++ b/x/staking/client/rest/query.go @@ -89,7 +89,7 @@ func registerQueryRoutes(cliCtx context.CLIContext, r *mux.Router) { // Get HistoricalInfo at a given height r.HandleFunc( - "/staking/historicalinfo/{height}", + "/staking/historical_info/{height}", historicalInfoHandlerFn(cliCtx), ).Methods("GET") diff --git a/x/staking/keeper/historical_info.go b/x/staking/keeper/historical_info.go index 898e23d4ba90..7d45eb7fbcba 100644 --- a/x/staking/keeper/historical_info.go +++ b/x/staking/keeper/historical_info.go @@ -6,7 +6,7 @@ import ( ) // GetHistoricalInfo gets the historical info at a given height -func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (hi types.HistoricalInfo, found bool) { +func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (types.HistoricalInfo, bool) { store := ctx.KVStore(k.storeKey) key := types.GetHistoricalInfoKey(height) @@ -16,7 +16,7 @@ func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (hi types.Histo return types.HistoricalInfo{}, false } - hi = types.MustUnmarshalHistoricalInfo(k.cdc, value) + hi := types.MustUnmarshalHistoricalInfo(k.cdc, value) return hi, true } diff --git a/x/staking/keeper/historical_info_test.go b/x/staking/keeper/historical_info_test.go index fb77ae952e76..ebdeea5370ec 100644 --- a/x/staking/keeper/historical_info_test.go +++ b/x/staking/keeper/historical_info_test.go @@ -11,10 +11,10 @@ import ( func TestHistoricalInfo(t *testing.T) { ctx, _, keeper, _ := CreateTestInput(t, false, 10) - var validators []types.Validator + validators := make([]types.Validator, len(addrVals)) for i, valAddr := range addrVals { - validators = append(validators, types.NewValidator(valAddr, PKs[i], types.Description{})) + validators[i] = types.NewValidator(valAddr, PKs[i], types.Description{}) } hi := types.NewHistoricalInfo(ctx.BlockHeader(), validators) diff --git a/x/staking/keeper/querier_test.go b/x/staking/keeper/querier_test.go index b91c92a06423..320266ca1a28 100644 --- a/x/staking/keeper/querier_test.go +++ b/x/staking/keeper/querier_test.go @@ -46,63 +46,63 @@ func TestNewQuerier(t *testing.T) { querier := NewQuerier(keeper) bz, err := querier(ctx, []string{"other"}, query) - require.NotNil(t, err) + require.Error(t, err) require.Nil(t, bz) _, err = querier(ctx, []string{"pool"}, query) - require.Nil(t, err) + require.NoError(t, err) _, err = querier(ctx, []string{"parameters"}, query) - require.Nil(t, err) + require.NoError(t, err) queryValParams := types.NewQueryValidatorParams(addrVal1) bz, errRes := cdc.MarshalJSON(queryValParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query.Path = "/custom/staking/validator" query.Data = bz _, err = querier(ctx, []string{"validator"}, query) - require.Nil(t, err) + require.NoError(t, err) _, err = querier(ctx, []string{"validatorDelegations"}, query) - require.Nil(t, err) + require.NoError(t, err) _, err = querier(ctx, []string{"validatorUnbondingDelegations"}, query) - require.Nil(t, err) + require.NoError(t, err) queryDelParams := types.NewQueryDelegatorParams(addrAcc2) bz, errRes = cdc.MarshalJSON(queryDelParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query.Path = "/custom/staking/validator" query.Data = bz _, err = querier(ctx, []string{"delegatorDelegations"}, query) - require.Nil(t, err) + require.NoError(t, err) _, err = querier(ctx, []string{"delegatorUnbondingDelegations"}, query) - require.Nil(t, err) + require.NoError(t, err) _, err = querier(ctx, []string{"delegatorValidators"}, query) - require.Nil(t, err) + require.NoError(t, err) bz, errRes = cdc.MarshalJSON(types.NewQueryRedelegationParams(nil, nil, nil)) - require.Nil(t, errRes) + require.NoError(t, errRes) query.Data = bz _, err = querier(ctx, []string{"redelegations"}, query) - require.Nil(t, err) + require.NoError(t, err) queryHisParams := types.NewQueryHistoricalInfoParams(5) bz, errRes = cdc.MarshalJSON(queryHisParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query.Path = "/custom/staking/historicalInfo" query.Data = bz _, err = querier(ctx, []string{"historicalInfo"}, query) - require.Nil(t, err) + require.NoError(t, err) } func TestQueryParametersPool(t *testing.T) { @@ -111,21 +111,21 @@ func TestQueryParametersPool(t *testing.T) { bondDenom := sdk.DefaultBondDenom res, err := queryParameters(ctx, keeper) - require.Nil(t, err) + require.NoError(t, err) var params types.Params errRes := cdc.UnmarshalJSON(res, ¶ms) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, keeper.GetParams(ctx), params) res, err = queryPool(ctx, keeper) - require.Nil(t, err) + require.NoError(t, err) var pool types.Pool bondedPool := keeper.GetBondedPool(ctx) notBondedPool := keeper.GetNotBondedPool(ctx) errRes = cdc.UnmarshalJSON(res, &pool) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, bondedPool.GetCoins().AmountOf(bondDenom), pool.BondedTokens) require.Equal(t, notBondedPool.GetCoins().AmountOf(bondDenom), pool.NotBondedTokens) } @@ -155,7 +155,7 @@ func TestQueryValidators(t *testing.T) { for i, s := range status { queryValsParams := types.NewQueryValidatorsParams(1, int(params.MaxValidators), s.String()) bz, err := cdc.MarshalJSON(queryValsParams) - require.Nil(t, err) + require.NoError(t, err) req := abci.RequestQuery{ Path: fmt.Sprintf("/custom/%s/%s", types.QuerierRoute, types.QueryValidators), @@ -163,11 +163,11 @@ func TestQueryValidators(t *testing.T) { } res, err := queryValidators(ctx, req, keeper) - require.Nil(t, err) + require.NoError(t, err) var validatorsResp []types.Validator err = cdc.UnmarshalJSON(res, &validatorsResp) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, 1, len(validatorsResp)) require.ElementsMatch(t, validators[i].OperatorAddress, validatorsResp[0].OperatorAddress) @@ -177,18 +177,18 @@ func TestQueryValidators(t *testing.T) { // Query each validator queryParams := types.NewQueryValidatorParams(addrVal1) bz, err := cdc.MarshalJSON(queryParams) - require.Nil(t, err) + require.NoError(t, err) query := abci.RequestQuery{ Path: "/custom/staking/validator", Data: bz, } res, err := queryValidator(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var validator types.Validator err = cdc.UnmarshalJSON(res, &validator) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, queriedValidators[0], validator) } @@ -216,7 +216,7 @@ func TestQueryDelegation(t *testing.T) { // Query Delegator bonded validators queryParams := types.NewQueryDelegatorParams(addrAcc2) bz, errRes := cdc.MarshalJSON(queryParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query := abci.RequestQuery{ Path: "/custom/staking/delegatorValidators", @@ -226,11 +226,11 @@ func TestQueryDelegation(t *testing.T) { delValidators := keeper.GetDelegatorValidators(ctx, addrAcc2, params.MaxValidators) res, err := queryDelegatorValidators(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var validatorsResp []types.Validator errRes = cdc.UnmarshalJSON(res, &validatorsResp) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, len(delValidators), len(validatorsResp)) require.ElementsMatch(t, delValidators, validatorsResp) @@ -239,12 +239,12 @@ func TestQueryDelegation(t *testing.T) { query.Data = bz[:len(bz)-1] _, err = queryDelegatorValidators(ctx, query, keeper) - require.NotNil(t, err) + require.Error(t, err) // Query bonded validator queryBondParams := types.NewQueryBondsParams(addrAcc2, addrVal1) bz, errRes = cdc.MarshalJSON(queryBondParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/delegatorValidator", @@ -252,11 +252,11 @@ func TestQueryDelegation(t *testing.T) { } res, err = queryDelegatorValidator(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var validator types.Validator errRes = cdc.UnmarshalJSON(res, &validator) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, delValidators[0], validator) @@ -264,7 +264,7 @@ func TestQueryDelegation(t *testing.T) { query.Data = bz[:len(bz)-1] _, err = queryDelegatorValidator(ctx, query, keeper) - require.NotNil(t, err) + require.Error(t, err) // Query delegation @@ -277,11 +277,11 @@ func TestQueryDelegation(t *testing.T) { require.True(t, found) res, err = queryDelegation(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var delegationRes types.DelegationResponse errRes = cdc.UnmarshalJSON(res, &delegationRes) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, delegation.ValidatorAddress, delegationRes.ValidatorAddress) require.Equal(t, delegation.DelegatorAddress, delegationRes.DelegatorAddress) @@ -294,11 +294,11 @@ func TestQueryDelegation(t *testing.T) { } res, err = queryDelegatorDelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var delegatorDelegations types.DelegationResponses errRes = cdc.UnmarshalJSON(res, &delegatorDelegations) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Len(t, delegatorDelegations, 1) require.Equal(t, delegation.ValidatorAddress, delegatorDelegations[0].ValidatorAddress) require.Equal(t, delegation.DelegatorAddress, delegatorDelegations[0].DelegatorAddress) @@ -308,12 +308,12 @@ func TestQueryDelegation(t *testing.T) { query.Data = bz[:len(bz)-1] _, err = queryDelegation(ctx, query, keeper) - require.NotNil(t, err) + require.Error(t, err) // Query validator delegations bz, errRes = cdc.MarshalJSON(types.NewQueryValidatorParams(addrVal1)) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "custom/staking/validatorDelegations", @@ -321,11 +321,11 @@ func TestQueryDelegation(t *testing.T) { } res, err = queryValidatorDelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var delegationsRes types.DelegationResponses errRes = cdc.UnmarshalJSON(res, &delegationsRes) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Len(t, delegatorDelegations, 1) require.Equal(t, delegation.ValidatorAddress, delegationsRes[0].ValidatorAddress) require.Equal(t, delegation.DelegatorAddress, delegationsRes[0].DelegatorAddress) @@ -334,11 +334,11 @@ func TestQueryDelegation(t *testing.T) { // Query unbonging delegation unbondingTokens := sdk.TokensFromConsensusPower(10) _, err = keeper.Undelegate(ctx, addrAcc2, val1.OperatorAddress, unbondingTokens.ToDec()) - require.Nil(t, err) + require.NoError(t, err) queryBondParams = types.NewQueryBondsParams(addrAcc2, addrVal1) bz, errRes = cdc.MarshalJSON(queryBondParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/unbondingDelegation", @@ -349,11 +349,11 @@ func TestQueryDelegation(t *testing.T) { require.True(t, found) res, err = queryUnbondingDelegation(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var unbondRes types.UnbondingDelegation errRes = cdc.UnmarshalJSON(res, &unbondRes) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, unbond, unbondRes) @@ -361,7 +361,7 @@ func TestQueryDelegation(t *testing.T) { query.Data = bz[:len(bz)-1] _, err = queryUnbondingDelegation(ctx, query, keeper) - require.NotNil(t, err) + require.Error(t, err) // Query Delegator Delegations @@ -371,29 +371,29 @@ func TestQueryDelegation(t *testing.T) { } res, err = queryDelegatorUnbondingDelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var delegatorUbds []types.UnbondingDelegation errRes = cdc.UnmarshalJSON(res, &delegatorUbds) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Equal(t, unbond, delegatorUbds[0]) // error unknown request query.Data = bz[:len(bz)-1] _, err = queryDelegatorUnbondingDelegations(ctx, query, keeper) - require.NotNil(t, err) + require.Error(t, err) // Query redelegation redelegationTokens := sdk.TokensFromConsensusPower(10) _, err = keeper.BeginRedelegation(ctx, addrAcc2, val1.OperatorAddress, val2.OperatorAddress, redelegationTokens.ToDec()) - require.Nil(t, err) + require.NoError(t, err) redel, found := keeper.GetRedelegation(ctx, addrAcc2, val1.OperatorAddress, val2.OperatorAddress) require.True(t, found) bz, errRes = cdc.MarshalJSON(types.NewQueryRedelegationParams(addrAcc2, val1.OperatorAddress, val2.OperatorAddress)) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/redelegations", @@ -401,11 +401,11 @@ func TestQueryDelegation(t *testing.T) { } res, err = queryRedelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var redelRes types.RedelegationResponses errRes = cdc.UnmarshalJSON(res, &redelRes) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Len(t, redelRes, 1) require.Equal(t, redel.DelegatorAddress, redelRes[0].DelegatorAddress) require.Equal(t, redel.ValidatorSrcAddress, redelRes[0].ValidatorSrcAddress) @@ -437,7 +437,7 @@ func TestQueryRedelegations(t *testing.T) { // delegator redelegations queryDelegatorParams := types.NewQueryDelegatorParams(addrAcc2) bz, errRes := cdc.MarshalJSON(queryDelegatorParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query := abci.RequestQuery{ Path: "/custom/staking/redelegations", @@ -445,11 +445,11 @@ func TestQueryRedelegations(t *testing.T) { } res, err := queryRedelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) var redelRes types.RedelegationResponses errRes = cdc.UnmarshalJSON(res, &redelRes) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Len(t, redelRes, 1) require.Equal(t, redel.DelegatorAddress, redelRes[0].DelegatorAddress) require.Equal(t, redel.ValidatorSrcAddress, redelRes[0].ValidatorSrcAddress) @@ -459,7 +459,7 @@ func TestQueryRedelegations(t *testing.T) { // validator redelegations queryValidatorParams := types.NewQueryValidatorParams(val1.GetOperator()) bz, errRes = cdc.MarshalJSON(queryValidatorParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/redelegations", @@ -467,10 +467,10 @@ func TestQueryRedelegations(t *testing.T) { } res, err = queryRedelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) errRes = cdc.UnmarshalJSON(res, &redelRes) - require.Nil(t, errRes) + require.NoError(t, errRes) require.Len(t, redelRes, 1) require.Equal(t, redel.DelegatorAddress, redelRes[0].DelegatorAddress) require.Equal(t, redel.ValidatorSrcAddress, redelRes[0].ValidatorSrcAddress) @@ -506,13 +506,13 @@ func TestQueryUnbondingDelegation(t *testing.T) { // queryValidatorParams := types.NewQueryBondsParams(addrAcc1, val1.GetOperator()) bz, errRes := cdc.MarshalJSON(queryValidatorParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query := abci.RequestQuery{ Path: "/custom/staking/unbondingDelegation", Data: bz, } res, err := queryUnbondingDelegation(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, res) var ubDel types.UnbondingDelegation require.NoError(t, cdc.UnmarshalJSON(res, &ubDel)) @@ -525,26 +525,26 @@ func TestQueryUnbondingDelegation(t *testing.T) { // queryValidatorParams = types.NewQueryBondsParams(addrAcc2, val1.GetOperator()) bz, errRes = cdc.MarshalJSON(queryValidatorParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/unbondingDelegation", Data: bz, } _, err = queryUnbondingDelegation(ctx, query, keeper) - require.NotNil(t, err) + require.Error(t, err) // // found: query unbonding delegation by delegator and validator // queryDelegatorParams := types.NewQueryDelegatorParams(addrAcc1) bz, errRes = cdc.MarshalJSON(queryDelegatorParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/delegatorUnbondingDelegations", Data: bz, } res, err = queryDelegatorUnbondingDelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, res) var ubDels []types.UnbondingDelegation require.NoError(t, cdc.UnmarshalJSON(res, &ubDels)) @@ -557,13 +557,13 @@ func TestQueryUnbondingDelegation(t *testing.T) { // queryDelegatorParams = types.NewQueryDelegatorParams(addrAcc2) bz, errRes = cdc.MarshalJSON(queryDelegatorParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query = abci.RequestQuery{ Path: "/custom/staking/delegatorUnbondingDelegations", Data: bz, } res, err = queryDelegatorUnbondingDelegations(ctx, query, keeper) - require.Nil(t, err) + require.NoError(t, err) require.NotNil(t, res) require.NoError(t, cdc.UnmarshalJSON(res, &ubDels)) require.Equal(t, 0, len(ubDels)) @@ -589,21 +589,21 @@ func TestQueryHistoricalInfo(t *testing.T) { queryHistoricalParams := types.NewQueryHistoricalInfoParams(4) bz, errRes := cdc.MarshalJSON(queryHistoricalParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query := abci.RequestQuery{ Path: "/custom/staking/historicalInfo", Data: bz, } res, err := queryHistoricalInfo(ctx, query, keeper) - require.NotNil(t, err, "Invalid query passed") + require.Error(t, err, "Invalid query passed") require.Nil(t, res, "Invalid query returned non-nil result") queryHistoricalParams = types.NewQueryHistoricalInfoParams(5) bz, errRes = cdc.MarshalJSON(queryHistoricalParams) - require.Nil(t, errRes) + require.NoError(t, errRes) query.Data = bz res, err = queryHistoricalInfo(ctx, query, keeper) - require.Nil(t, err, "Valid query passed") + require.NoError(t, err, "Valid query passed") require.NotNil(t, res, "Valid query returned nil result") var recv types.HistoricalInfo diff --git a/x/staking/types/historical_info.go b/x/staking/types/historical_info.go index e3190a40ce76..2924054ca6d1 100644 --- a/x/staking/types/historical_info.go +++ b/x/staking/types/historical_info.go @@ -11,8 +11,8 @@ import ( // HistoricalInfo contains the historical information that gets stored at each height type HistoricalInfo struct { - Header abci.Header - ValSet []Validator + Header abci.Header `json:"header" yaml:"header"` + ValSet []Validator `json:"valset" yaml:"valset"` } //NewHistoricalInfo will create a historical information struct from header and valset @@ -45,9 +45,9 @@ func UnmarshalHistoricalInfo(cdc *codec.Codec, value []byte) (hi HistoricalInfo, return hi, err } -// ValidateHistoricalInfo will ensure HistoricalInfo is not nil and sorted -func ValidateHistoricalInfo(hi HistoricalInfo) error { - if hi.ValSet != nil { +// ValidateBasic will ensure HistoricalInfo is not nil and sorted +func ValidateBasic(hi HistoricalInfo) error { + if len(hi.ValSet) == 0 { return sdkerrors.Wrap(ErrInvalidHistoricalInfo(DefaultCodespace), "ValidatorSer is nil") } if !sort.IsSorted(Validators(hi.ValSet)) { diff --git a/x/staking/types/historical_info_test.go b/x/staking/types/historical_info_test.go index e2c455cb8054..f607ba15031c 100644 --- a/x/staking/types/historical_info_test.go +++ b/x/staking/types/historical_info_test.go @@ -1,6 +1,7 @@ package types import ( + "math/rand" "sort" "testing" @@ -8,16 +9,19 @@ import ( abci "github.com/tendermint/tendermint/abci/types" ) -func TestHistoricalInfo(t *testing.T) { - validators := []Validator{ +var ( + validators = []Validator{ NewValidator(valAddr1, pk1, Description{}), NewValidator(valAddr2, pk2, Description{}), NewValidator(valAddr3, pk3, Description{}), } - header := abci.Header{ + header = abci.Header{ ChainID: "hello", Height: 5, } +) + +func TestHistoricalInfo(t *testing.T) { hi := NewHistoricalInfo(header, validators) require.True(t, sort.IsSorted(Validators(hi.ValSet)), "Validators are not sorted") @@ -33,3 +37,31 @@ func TestHistoricalInfo(t *testing.T) { require.Equal(t, hi, recv, "Unmarshalled HistoricalInfo is different from original") require.True(t, sort.IsSorted(Validators(hi.ValSet)), "Validators are not sorted") } + +func TestValidateBasic(t *testing.T) { + hi := HistoricalInfo{ + Header: header, + } + err := ValidateBasic(hi) + require.Error(t, err, "ValidateBasic passed on nil ValSet") + + // Ensure validators are not sorted + for sort.IsSorted(Validators(validators)) { + rand.Shuffle(len(validators), func(i, j int) { + it := validators[i] + validators[i] = validators[j] + validators[j] = it + }) + } + + hi = HistoricalInfo{ + Header: header, + ValSet: validators, + } + err = ValidateBasic(hi) + require.Error(t, err, "ValidateBasic passed on unsorted ValSet") + + hi = NewHistoricalInfo(header, validators) + err = ValidateBasic(hi) + require.NoError(t, err, "ValidateBasic failed on valid HistoricalInfo") +} diff --git a/x/staking/types/params.go b/x/staking/types/params.go index 04462732ea82..f3aefb404899 100644 --- a/x/staking/types/params.go +++ b/x/staking/types/params.go @@ -25,6 +25,8 @@ const ( // Default maximum entries in a UBD/RED pair DefaultMaxEntries uint16 = 7 + // DefaultHistorical entries is 0 since it must only be non-zero for + // IBC connected chains DefaultHistoricalEntries uint16 = 0 ) From a45a5fa2d9a0c63aab7af8294c9e0f0e3e1a48e0 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 13 Dec 2019 10:58:55 -0800 Subject: [PATCH 11/19] appease linter --- x/staking/client/cli/query.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/staking/client/cli/query.go b/x/staking/client/cli/query.go index c2d91d4f0b70..1a9b06a06e38 100644 --- a/x/staking/client/cli/query.go +++ b/x/staking/client/cli/query.go @@ -1,7 +1,6 @@ package cli import ( - "errors" "fmt" "strconv" "strings" @@ -550,7 +549,7 @@ $ %s query staking historical-info 5 height, err := strconv.ParseInt(args[0], 10, 64) if err != nil || height < 0 { - return errors.New(fmt.Sprintf("Height argument provided must be a non-negative-integer: %v", err)) + return fmt.Errorf("Height argument provided must be a non-negative-integer: %v", err) } bz, err := cdc.MarshalJSON(types.QueryHistoricalInfoParams{Height: height}) From ef43df5f8d11b69cc46c0af6997fcccb695fc842 Mon Sep 17 00:00:00 2001 From: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Date: Mon, 16 Dec 2019 12:00:33 -0300 Subject: [PATCH 12/19] Update x/staking/client/cli/query.go --- x/staking/client/cli/query.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/staking/client/cli/query.go b/x/staking/client/cli/query.go index 1a9b06a06e38..632237c1d915 100644 --- a/x/staking/client/cli/query.go +++ b/x/staking/client/cli/query.go @@ -549,7 +549,7 @@ $ %s query staking historical-info 5 height, err := strconv.ParseInt(args[0], 10, 64) if err != nil || height < 0 { - return fmt.Errorf("Height argument provided must be a non-negative-integer: %v", err) + return fmt.Errorf("height argument provided must be a non-negative-integer: %v", err) } bz, err := cdc.MarshalJSON(types.QueryHistoricalInfoParams{Height: height}) From c3fc30b6d4a444b1199cb29da2c4b38c89184cc8 Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 16 Dec 2019 10:46:38 -0800 Subject: [PATCH 13/19] Apply suggestions from code review Co-Authored-By: Alexander Bezobchuk --- x/staking/abci.go | 8 ++++---- x/staking/types/historical_info.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/staking/abci.go b/x/staking/abci.go index 16b893559421..fb5b83d15253 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -24,13 +24,13 @@ func BeginBlocker(ctx sdk.Context, k Keeper) { // Set latest HistoricalInfo at current height k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) - // prune store to ensure we only have parameter-defined historical entries - // in most cases, this will involve removing a single historical entry + // Prune store to ensure we only have parameter-defined historical entries. + // In most cases, this will involve removing a single historical entry. // In the rare scenario when the historical entries gets reduced to a lower value k' - // from the original value k. k - k' entries must be deleted from the store + // from the original value k. k - k' entries must be deleted from the store. // Since the entries to be deleted are always in a continuous range, we can iterate // over the historical entries starting from the most recent version to be pruned - // and then return at the first empty entry + // and then return at the first empty entry. for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- { _, found := k.GetHistoricalInfo(ctx, i) if found { diff --git a/x/staking/types/historical_info.go b/x/staking/types/historical_info.go index 2924054ca6d1..8fae88f07f4e 100644 --- a/x/staking/types/historical_info.go +++ b/x/staking/types/historical_info.go @@ -15,7 +15,7 @@ type HistoricalInfo struct { ValSet []Validator `json:"valset" yaml:"valset"` } -//NewHistoricalInfo will create a historical information struct from header and valset +// NewHistoricalInfo will create a historical information struct from header and valset // it will first sort valset before inclusion into historical info func NewHistoricalInfo(header abci.Header, valSet []Validator) HistoricalInfo { sort.Sort(Validators(valSet)) From e4d2c3455c47032f73731e427164fa6ad93603c3 Mon Sep 17 00:00:00 2001 From: Aditya Date: Mon, 16 Dec 2019 11:21:08 -0800 Subject: [PATCH 14/19] Update x/staking/keeper/historical_info.go Co-Authored-By: Alexander Bezobchuk --- x/staking/keeper/historical_info.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/staking/keeper/historical_info.go b/x/staking/keeper/historical_info.go index 7d45eb7fbcba..5a71a78bce46 100644 --- a/x/staking/keeper/historical_info.go +++ b/x/staking/keeper/historical_info.go @@ -11,7 +11,6 @@ func (k Keeper) GetHistoricalInfo(ctx sdk.Context, height int64) (types.Historic key := types.GetHistoricalInfoKey(height) value := store.Get(key) - if value == nil { return types.HistoricalInfo{}, false } From 74b3f19d34b5b4247b0b1a9704fb96c13fea8604 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 16 Dec 2019 16:16:15 -0800 Subject: [PATCH 15/19] update spec, alias, and fix minor bug --- x/staking/abci.go | 85 +++++++++++++++---- x/staking/alias.go | 13 +++ x/staking/handler.go | 56 ------------ x/staking/spec/01_state.md | 18 ++++ x/staking/spec/04_begin_block.md | 16 ++++ .../spec/{04_end_block.md => 05_end_block.md} | 2 +- x/staking/spec/{05_hooks.md => 06_hooks.md} | 2 +- x/staking/spec/{06_events.md => 07_events.md} | 2 +- x/staking/spec/07_params.md | 14 --- x/staking/spec/08_params.md | 15 ++++ x/staking/spec/README.md | 19 +++-- x/staking/types/keys.go | 4 +- 12 files changed, 148 insertions(+), 98 deletions(-) create mode 100644 x/staking/spec/04_begin_block.md rename x/staking/spec/{04_end_block.md => 05_end_block.md} (99%) rename x/staking/spec/{05_hooks.md => 06_hooks.md} (99%) rename x/staking/spec/{06_events.md => 07_events.md} (99%) delete mode 100644 x/staking/spec/07_params.md create mode 100644 x/staking/spec/08_params.md diff --git a/x/staking/abci.go b/x/staking/abci.go index fb5b83d15253..58dae0cea073 100644 --- a/x/staking/abci.go +++ b/x/staking/abci.go @@ -2,27 +2,15 @@ package staking import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/cosmos-sdk/x/staking/types" + abci "github.com/tendermint/tendermint/abci/types" ) // BeginBlocker will persist the current header and validator set as a historical entry // and prune the oldest entry based on the HistoricalEntries parameter func BeginBlocker(ctx sdk.Context, k Keeper) { entryNum := k.HistoricalEntries(ctx) - // if there is no need to persist historicalInfo, return - if entryNum == 0 { - return - } - - // Create HistoricalInfo struct - lastVals := k.GetLastValidators(ctx) - historicalEntry := types.NewHistoricalInfo( - ctx.BlockHeader(), - lastVals, - ) - - // Set latest HistoricalInfo at current height - k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) // Prune store to ensure we only have parameter-defined historical entries. // In most cases, this will involve removing a single historical entry. @@ -36,7 +24,74 @@ func BeginBlocker(ctx sdk.Context, k Keeper) { if found { k.DeleteHistoricalInfo(ctx, i) } else { - return + break } } + + // if there is no need to persist historicalInfo, return + if entryNum == 0 { + return + } + + // Create HistoricalInfo struct + lastVals := k.GetLastValidators(ctx) + historicalEntry := types.NewHistoricalInfo(ctx.BlockHeader(), lastVals) + + // Set latest HistoricalInfo at current height + k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) +} + +// Called every block, update validator set +func EndBlocker(ctx sdk.Context, k keeper.Keeper) []abci.ValidatorUpdate { + // Calculate validator set changes. + // + // NOTE: ApplyAndReturnValidatorSetUpdates has to come before + // UnbondAllMatureValidatorQueue. + // This fixes a bug when the unbonding period is instant (is the case in + // some of the tests). The test expected the validator to be completely + // unbonded after the Endblocker (go from Bonded -> Unbonding during + // ApplyAndReturnValidatorSetUpdates and then Unbonding -> Unbonded during + // UnbondAllMatureValidatorQueue). + validatorUpdates := k.ApplyAndReturnValidatorSetUpdates(ctx) + + // Unbond all mature validators from the unbonding queue. + k.UnbondAllMatureValidatorQueue(ctx) + + // Remove all mature unbonding delegations from the ubd queue. + matureUnbonds := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time) + for _, dvPair := range matureUnbonds { + err := k.CompleteUnbonding(ctx, dvPair.DelegatorAddress, dvPair.ValidatorAddress) + if err != nil { + continue + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeCompleteUnbonding, + sdk.NewAttribute(types.AttributeKeyValidator, dvPair.ValidatorAddress.String()), + sdk.NewAttribute(types.AttributeKeyDelegator, dvPair.DelegatorAddress.String()), + ), + ) + } + + // Remove all mature redelegations from the red queue. + matureRedelegations := k.DequeueAllMatureRedelegationQueue(ctx, ctx.BlockHeader().Time) + for _, dvvTriplet := range matureRedelegations { + err := k.CompleteRedelegation(ctx, dvvTriplet.DelegatorAddress, + dvvTriplet.ValidatorSrcAddress, dvvTriplet.ValidatorDstAddress) + if err != nil { + continue + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeCompleteRedelegation, + sdk.NewAttribute(types.AttributeKeyDelegator, dvvTriplet.DelegatorAddress.String()), + sdk.NewAttribute(types.AttributeKeySrcValidator, dvvTriplet.ValidatorSrcAddress.String()), + sdk.NewAttribute(types.AttributeKeyDstValidator, dvvTriplet.ValidatorDstAddress.String()), + ), + ) + } + + return validatorUpdates } diff --git a/x/staking/alias.go b/x/staking/alias.go index 019fa0298532..887b9b7bf690 100644 --- a/x/staking/alias.go +++ b/x/staking/alias.go @@ -19,6 +19,7 @@ const ( CodeInvalidDelegation = types.CodeInvalidDelegation CodeInvalidInput = types.CodeInvalidInput CodeValidatorJailed = types.CodeValidatorJailed + CodeInvalidHistoricalInfo = types.CodeInvalidHistoricalInfo CodeInvalidAddress = types.CodeInvalidAddress CodeUnauthorized = types.CodeUnauthorized CodeInternal = types.CodeInternal @@ -47,6 +48,7 @@ const ( QueryDelegatorValidator = types.QueryDelegatorValidator QueryPool = types.QueryPool QueryParameters = types.QueryParameters + QueryHistoricalInfo = types.QueryHistoricalInfo MaxMonikerLength = types.MaxMonikerLength MaxIdentityLength = types.MaxIdentityLength MaxWebsiteLength = types.MaxWebsiteLength @@ -86,6 +88,10 @@ var ( NewDelegationResp = types.NewDelegationResp NewRedelegationResponse = types.NewRedelegationResponse NewRedelegationEntryResponse = types.NewRedelegationEntryResponse + NewHistoricalInfo = types.NewHistoricalInfo + MustMarshalHistoricalInfo = types.MustMarshalHistoricalInfo + MustUnmarshalHistoricalInfo = types.MustUnmarshalHistoricalInfo + UnmarshalHistoricalInfo = types.UnmarshalHistoricalInfo ErrNilValidatorAddr = types.ErrNilValidatorAddr ErrBadValidatorAddr = types.ErrBadValidatorAddr ErrNoValidatorFound = types.ErrNoValidatorFound @@ -131,6 +137,8 @@ var ( ErrBothShareMsgsGiven = types.ErrBothShareMsgsGiven ErrNeitherShareMsgsGiven = types.ErrNeitherShareMsgsGiven ErrMissingSignature = types.ErrMissingSignature + ErrInvalidHistoricalInfo = types.ErrInvalidHistoricalInfo + ErrNoHistoricalInfo = types.ErrNoHistoricalInfo NewGenesisState = types.NewGenesisState DefaultGenesisState = types.DefaultGenesisState NewMultiStakingHooks = types.NewMultiStakingHooks @@ -159,6 +167,7 @@ var ( GetREDsFromValSrcIndexKey = types.GetREDsFromValSrcIndexKey GetREDsToValDstIndexKey = types.GetREDsToValDstIndexKey GetREDsByDelToValDstIndexKey = types.GetREDsByDelToValDstIndexKey + GetHistoricalInfoKey = types.GetHistoricalInfoKey NewMsgCreateValidator = types.NewMsgCreateValidator NewMsgEditValidator = types.NewMsgEditValidator NewMsgDelegate = types.NewMsgDelegate @@ -174,6 +183,7 @@ var ( NewQueryBondsParams = types.NewQueryBondsParams NewQueryRedelegationParams = types.NewQueryRedelegationParams NewQueryValidatorsParams = types.NewQueryValidatorsParams + NewQueryHistoricalInfoParams = types.NewQueryHistoricalInfoParams NewValidator = types.NewValidator MustMarshalValidator = types.MustMarshalValidator MustUnmarshalValidator = types.MustUnmarshalValidator @@ -196,6 +206,7 @@ var ( UnbondingQueueKey = types.UnbondingQueueKey RedelegationQueueKey = types.RedelegationQueueKey ValidatorQueueKey = types.ValidatorQueueKey + HistoricalInfoKey = types.HistoricalInfoKey KeyUnbondingTime = types.KeyUnbondingTime KeyMaxValidators = types.KeyMaxValidators KeyMaxEntries = types.KeyMaxEntries @@ -216,6 +227,7 @@ type ( Redelegation = types.Redelegation RedelegationEntry = types.RedelegationEntry Redelegations = types.Redelegations + HistoricalInfo = types.HistoricalInfo DelegationResponse = types.DelegationResponse DelegationResponses = types.DelegationResponses RedelegationResponse = types.RedelegationResponse @@ -237,6 +249,7 @@ type ( QueryBondsParams = types.QueryBondsParams QueryRedelegationParams = types.QueryRedelegationParams QueryValidatorsParams = types.QueryValidatorsParams + QueryHistoricalInfoParams = types.QueryHistoricalInfoParams Validator = types.Validator Validators = types.Validators Description = types.Description diff --git a/x/staking/handler.go b/x/staking/handler.go index 1d7ba1e84762..376da255ea1c 100644 --- a/x/staking/handler.go +++ b/x/staking/handler.go @@ -4,7 +4,6 @@ import ( "fmt" "time" - abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/common" tmtypes "github.com/tendermint/tendermint/types" @@ -40,61 +39,6 @@ func NewHandler(k keeper.Keeper) sdk.Handler { } } -// Called every block, update validator set -func EndBlocker(ctx sdk.Context, k keeper.Keeper) []abci.ValidatorUpdate { - // Calculate validator set changes. - // - // NOTE: ApplyAndReturnValidatorSetUpdates has to come before - // UnbondAllMatureValidatorQueue. - // This fixes a bug when the unbonding period is instant (is the case in - // some of the tests). The test expected the validator to be completely - // unbonded after the Endblocker (go from Bonded -> Unbonding during - // ApplyAndReturnValidatorSetUpdates and then Unbonding -> Unbonded during - // UnbondAllMatureValidatorQueue). - validatorUpdates := k.ApplyAndReturnValidatorSetUpdates(ctx) - - // Unbond all mature validators from the unbonding queue. - k.UnbondAllMatureValidatorQueue(ctx) - - // Remove all mature unbonding delegations from the ubd queue. - matureUnbonds := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time) - for _, dvPair := range matureUnbonds { - err := k.CompleteUnbonding(ctx, dvPair.DelegatorAddress, dvPair.ValidatorAddress) - if err != nil { - continue - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeCompleteUnbonding, - sdk.NewAttribute(types.AttributeKeyValidator, dvPair.ValidatorAddress.String()), - sdk.NewAttribute(types.AttributeKeyDelegator, dvPair.DelegatorAddress.String()), - ), - ) - } - - // Remove all mature redelegations from the red queue. - matureRedelegations := k.DequeueAllMatureRedelegationQueue(ctx, ctx.BlockHeader().Time) - for _, dvvTriplet := range matureRedelegations { - err := k.CompleteRedelegation(ctx, dvvTriplet.DelegatorAddress, - dvvTriplet.ValidatorSrcAddress, dvvTriplet.ValidatorDstAddress) - if err != nil { - continue - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeCompleteRedelegation, - sdk.NewAttribute(types.AttributeKeyDelegator, dvvTriplet.DelegatorAddress.String()), - sdk.NewAttribute(types.AttributeKeySrcValidator, dvvTriplet.ValidatorSrcAddress.String()), - sdk.NewAttribute(types.AttributeKeyDstValidator, dvvTriplet.ValidatorDstAddress.String()), - ), - ) - } - - return validatorUpdates -} - // These functions assume everything has been authenticated, // now we just perform action and save diff --git a/x/staking/spec/01_state.md b/x/staking/spec/01_state.md index 890d6925118d..534fbd21bb70 100644 --- a/x/staking/spec/01_state.md +++ b/x/staking/spec/01_state.md @@ -282,3 +282,21 @@ The stored object as each key is an array of validator operator addresses from which the validator object can be accessed. Typically it is expected that only a single validator record will be associated with a given timestamp however it is possible that multiple validators exist in the queue at the same location. + +## HistoricalInfo + +HistoricalInfo objects are stored and pruned at each block such that the staking keeper persists +the `n` most recent historical info defined by staking module parameter: `HistoricalEntries`. + +```go +type HistoricalInfo struct { + Header abci.Header + ValSet []types.Validator +} +``` + +At each BeginBlock, the staking keeper will persist the current Header and the Validators that committed +the current block in a `HistoricalInfo` object. The Validators are sorted on their address to ensure that +they are in a determisnistic order. +The oldest HistoricalEntries will be pruned to ensure that there only exist the parameter-defined number of +historical entries. diff --git a/x/staking/spec/04_begin_block.md b/x/staking/spec/04_begin_block.md new file mode 100644 index 000000000000..96aa08520a60 --- /dev/null +++ b/x/staking/spec/04_begin_block.md @@ -0,0 +1,16 @@ +--- +order: 4 +--- + +# Begin-Block + +Each abci begin block call, the historical info will get stored and pruned +according to the `HistoricalEntries` parameter. + +## Historical Info Tracking + +If the `HistoricalEntries` parameter is 0, then the `BeginBlock` performs a no-op. + +Otherwise, the latest historical info is stored under the key `historicalInfoKey|height`, while any entries older than `height - HistoricalEntries` is deleted. +In most cases, this results in a single entry being pruned per block. +However, if the parameter `HistoricalEntries` has changed to a lower value there will be multiple entries in the store that must be pruned. diff --git a/x/staking/spec/04_end_block.md b/x/staking/spec/05_end_block.md similarity index 99% rename from x/staking/spec/04_end_block.md rename to x/staking/spec/05_end_block.md index d42b26704e9e..054bc87498b4 100644 --- a/x/staking/spec/04_end_block.md +++ b/x/staking/spec/05_end_block.md @@ -1,5 +1,5 @@ --- -order: 4 +order: 5 --- # End-Block diff --git a/x/staking/spec/05_hooks.md b/x/staking/spec/06_hooks.md similarity index 99% rename from x/staking/spec/05_hooks.md rename to x/staking/spec/06_hooks.md index 10e084e2a5f0..ac3c9147795c 100644 --- a/x/staking/spec/05_hooks.md +++ b/x/staking/spec/06_hooks.md @@ -1,5 +1,5 @@ --- -order: 5 +order: 6 --- # Hooks diff --git a/x/staking/spec/06_events.md b/x/staking/spec/07_events.md similarity index 99% rename from x/staking/spec/06_events.md rename to x/staking/spec/07_events.md index 5994af5fa3a6..4b8f58bbcd4f 100644 --- a/x/staking/spec/06_events.md +++ b/x/staking/spec/07_events.md @@ -1,5 +1,5 @@ --- -order: 6 +order: 7 --- # Events diff --git a/x/staking/spec/07_params.md b/x/staking/spec/07_params.md deleted file mode 100644 index 4fdd5ca124e4..000000000000 --- a/x/staking/spec/07_params.md +++ /dev/null @@ -1,14 +0,0 @@ ---- -order: 7 ---- - -# Parameters - -The staking module contains the following parameters: - -| Key | Type | Example | -|---------------|------------------|-------------------| -| UnbondingTime | string (time ns) | "259200000000000" | -| MaxValidators | uint16 | 100 | -| KeyMaxEntries | uint16 | 7 | -| BondDenom | string | "uatom" | diff --git a/x/staking/spec/08_params.md b/x/staking/spec/08_params.md new file mode 100644 index 000000000000..a0c5932c5f98 --- /dev/null +++ b/x/staking/spec/08_params.md @@ -0,0 +1,15 @@ +--- +order: 8 +--- + +# Parameters + +The staking module contains the following parameters: + +| Key | Type | Example | +|-------------------|------------------|-------------------| +| UnbondingTime | string (time ns) | "259200000000000" | +| MaxValidators | uint16 | 100 | +| KeyMaxEntries | uint16 | 7 | +| HistoricalEntries | uint16 | 3 | +| BondDenom | string | "uatom" | diff --git a/x/staking/spec/README.md b/x/staking/spec/README.md index c889d06841be..568f15ac7eea 100644 --- a/x/staking/spec/README.md +++ b/x/staking/spec/README.md @@ -32,6 +32,7 @@ network. - [UnbondingDelegation](01_state.md#unbondingdelegation) - [Redelegation](01_state.md#redelegation) - [Queues](01_state.md#queues) + - [HistoricalInfo](01_state.md#historicalinfo) 2. **[State Transitions](02_state_transitions.md)** - [Validators](02_state_transitions.md#validators) - [Delegations](02_state_transitions.md#delegations) @@ -42,11 +43,13 @@ network. - [MsgDelegate](03_messages.md#msgdelegate) - [MsgBeginUnbonding](03_messages.md#msgbeginunbonding) - [MsgBeginRedelegate](03_messages.md#msgbeginredelegate) -4. **[End-Block ](04_end_block.md)** - - [Validator Set Changes](04_end_block.md#validator-set-changes) - - [Queues ](04_end_block.md#queues-) -5. **[Hooks](05_hooks.md)** -6. **[Events](06_events.md)** - - [EndBlocker](06_events.md#endblocker) - - [Handlers](06_events.md#handlers) -7. **[Parameters](07_params.md)** +4. **[Begin-Block](04_begin_block.md)** + - [Historical Info Tracking](04_begin_block.md#historical-info-tracking) +4. **[End-Block ](05_end_block.md)** + - [Validator Set Changes](05_end_block.md#validator-set-changes) + - [Queues ](05_end_block.md#queues-) +5. **[Hooks](06_hooks.md)** +6. **[Events](07_events.md)** + - [EndBlocker](07_events.md#endblocker) + - [Handlers](07_events.md#handlers) +7. **[Parameters](08_params.md)** diff --git a/x/staking/types/keys.go b/x/staking/types/keys.go index 77b2a2d710f8..d278b50db3e7 100644 --- a/x/staking/types/keys.go +++ b/x/staking/types/keys.go @@ -2,7 +2,7 @@ package types import ( "encoding/binary" - "fmt" + "strconv" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -286,5 +286,5 @@ func GetREDsByDelToValDstIndexKey(delAddr sdk.AccAddress, valDstAddr sdk.ValAddr // GetHistoricalInfoKey gets the key for the historical info func GetHistoricalInfoKey(height int64) []byte { - return append(HistoricalInfoKey, []byte(fmt.Sprintf("%d", height))...) + return append(HistoricalInfoKey, []byte(strconv.FormatInt(height, 10))...) } From 73cbb7eb1856e09f0d9ef156a6f128fbbabbea24 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 16 Dec 2019 16:38:38 -0800 Subject: [PATCH 16/19] cleanup changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c7e894d47c..d28304f3d95c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -148,8 +148,8 @@ that allows for arbitrary vesting periods. * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks. * (cli) [\#5223](https://github.com/cosmos/cosmos-sdk/issues/5223) Cosmos Ledger App v2.0.0 is now supported. The changes are backwards compatible and App v1.5.x is still supported. * (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduced ability to store historical info entries in staking keeper, allows applications to introspect specified number of past headers and validator sets -* (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduces new parameter `HistoricalEntries` which allows applications to determine how many recent historical info entries they want to persist in store. Default value is 0. -* (x/staking) [\#5380](https://github.com/cosmos/cosmos-sdk/pull/5380) Introduces cli commands and rest routes to query historical information at a given height + * Introduces new parameter `HistoricalEntries` which allows applications to determine how many recent historical info entries they want to persist in store. Default value is 0. + * Introduces cli commands and rest routes to query historical information at a given height * (modules) [\#5249](https://github.com/cosmos/cosmos-sdk/pull/5249) Funds are now allowed to be directly sent to the community pool (via the distribution module account). * (keys) [\#4941](https://github.com/cosmos/cosmos-sdk/issues/4941) Introduce keybase option to allow overriding the default private key implementation of a key generated through the `keys add` cli command. From 9364ba43a4eebb757326605847299fd5b39cbfce Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 17 Dec 2019 10:45:36 -0800 Subject: [PATCH 17/19] Make BeginBlocker/EndBlocker methods on keeper --- x/staking/app_test.go | 2 +- x/staking/handler_test.go | 68 ++++++++++++++--------------- x/staking/{ => keeper}/abci.go | 7 ++- x/staking/{ => keeper}/abci_test.go | 17 ++++---- x/staking/module.go | 4 +- 5 files changed, 48 insertions(+), 50 deletions(-) rename x/staking/{ => keeper}/abci.go (94%) rename x/staking/{ => keeper}/abci_test.go (80%) diff --git a/x/staking/app_test.go b/x/staking/app_test.go index 0c2615d7a3ea..911128fc49b1 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -56,7 +56,7 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { // getEndBlocker returns a staking endblocker. func getEndBlocker(keeper Keeper) sdk.EndBlocker { return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { - validatorUpdates := EndBlocker(ctx, keeper) + validatorUpdates := keeper.EndBlocker(ctx) return abci.ResponseEndBlock{ ValidatorUpdates: validatorUpdates, diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index 0e999ba9b43b..f06cb0b3bd3b 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -91,8 +91,8 @@ func TestValidatorByPowerIndex(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) + keeper.EndBlocker(ctx) // verify that by power key nolonger exists _, found = keeper.GetValidator(ctx, validatorAddr) @@ -216,7 +216,7 @@ func TestLegacyValidatorDelegations(t *testing.T) { var finishTime time.Time types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // verify the validator record still exists, is jailed, and has correct tokens validator, found = keeper.GetValidator(ctx, valAddr) @@ -438,7 +438,7 @@ func TestIncrementsMsgUnbond(t *testing.T) { var finishTime time.Time types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // check that the accounts and the bond account have the appropriate values validator, found = keeper.GetValidator(ctx, validatorAddr) @@ -532,7 +532,7 @@ func TestMultipleMsgCreateValidator(t *testing.T) { require.Equal(t, balanceExpd, balanceGot, "expected account to have %d, got %d", balanceExpd, balanceGot) } - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // unbond them all by removing delegation for i, validatorAddr := range validatorAddrs { @@ -548,10 +548,10 @@ func TestMultipleMsgCreateValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) // adds validator into unbonding queue - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // removes validator from queue and set - EndBlocker(ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)), keeper) + keeper.EndBlocker(ctx.WithBlockTime(blockTime.Add(params.UnbondingTime))) // Check that the validator is deleted from state validators := keeper.GetValidators(ctx, 100) @@ -599,7 +599,7 @@ func TestMultipleMsgDelegate(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // check that the account is unbonded _, found := keeper.GetDelegation(ctx, delegatorAddr, validatorAddr) @@ -631,7 +631,7 @@ func TestJailValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) validator, found := keeper.GetValidator(ctx, validatorAddr) require.True(t, found) @@ -645,7 +645,7 @@ func TestJailValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // verify that the pubkey can now be reused got = handleMsgCreateValidator(ctx, msgCreateValidator, keeper) @@ -673,7 +673,7 @@ func TestValidatorQueue(t *testing.T) { got = handleMsgDelegate(ctx, msgDelegate, keeper) require.True(t, got.IsOK(), "expected ok, got %v", got) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // unbond the all self-delegation to put validator in unbonding state unbondAmt := sdk.NewCoin(sdk.DefaultBondDenom, delTokens) @@ -685,7 +685,7 @@ func TestValidatorQueue(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) origHeader := ctx.BlockHeader() @@ -695,7 +695,7 @@ func TestValidatorQueue(t *testing.T) { // should still be unbonding at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) validator, found = keeper.GetValidator(ctx, validatorAddr) require.True(t, found) @@ -703,7 +703,7 @@ func TestValidatorQueue(t *testing.T) { // should be in unbonded state at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) validator, found = keeper.GetValidator(ctx, validatorAddr) require.True(t, found) @@ -725,7 +725,7 @@ func TestUnbondingPeriod(t *testing.T) { got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // begin unbonding unbondAmt := sdk.NewCoin(sdk.DefaultBondDenom, sdk.TokensFromConsensusPower(10)) @@ -739,19 +739,19 @@ func TestUnbondingPeriod(t *testing.T) { require.True(t, found, "should not have unbonded") // cannot complete unbonding at same time - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.True(t, found, "should not have unbonded") // cannot complete unbonding at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.True(t, found, "should not have unbonded") // can complete unbonding at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.False(t, found, "should have unbonded") } @@ -789,7 +789,7 @@ func TestUnbondingFromUnbondingValidator(t *testing.T) { ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(keeper.UnbondingTime(ctx))) // Run the EndBlocker - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // Check to make sure that the unbonding delegation is no longer in state // (meaning it was deleted in the above EndBlocker) @@ -839,19 +839,19 @@ func TestRedelegationPeriod(t *testing.T) { origHeader := ctx.BlockHeader() // cannot complete redelegation at same time - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) _, found := keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.True(t, found, "should not have unbonded") // cannot complete redelegation at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) _, found = keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.True(t, found, "should not have unbonded") // can complete redelegation at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) _, found = keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.False(t, found, "should have unbonded") } @@ -893,7 +893,7 @@ func TestTransitiveRedelegation(t *testing.T) { ctx = ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)) // complete first redelegation - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // now should be able to redelegate from the second validator to the third got = handleMsgBeginRedelegate(ctx, msgBeginRedelegate, keeper) @@ -921,7 +921,7 @@ func TestMultipleRedelegationAtSameTime(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond them - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // begin a redelegate selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -946,7 +946,7 @@ func TestMultipleRedelegationAtSameTime(t *testing.T) { // move forward in time, should complete both redelegations ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(1 * time.Second)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.False(t, found) @@ -973,7 +973,7 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond them - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // begin a redelegate selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -994,14 +994,14 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { // move forward in time, should complete the first redelegation, but not the second ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.True(t, found) require.Len(t, rd.Entries, 1) // move forward in time, should complete the second redelegation ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.False(t, found) } @@ -1022,7 +1022,7 @@ func TestMultipleUnbondingDelegationAtSameTime(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // begin an unbonding delegation selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -1047,7 +1047,7 @@ func TestMultipleUnbondingDelegationAtSameTime(t *testing.T) { // move forwaubd in time, should complete both ubds ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(1 * time.Second)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.False(t, found) @@ -1069,7 +1069,7 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // begin an unbonding delegation selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -1095,14 +1095,14 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { // move forwaubd in time, should complete the first redelegation, but not the second ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.True(t, found) require.Len(t, ubd.Entries, 1) // move forwaubd in time, should complete the second redelegation ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.False(t, found) } @@ -1254,7 +1254,7 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) { require.Equal(t, sdk.NewDecFromInt(redAmt.Amount.QuoRaw(2)), delegation.Shares) // end blocker - EndBlocker(ctx, keeper) + keeper.EndBlocker(ctx) // validator power should have been reduced to zero // validator should be in unbonding state diff --git a/x/staking/abci.go b/x/staking/keeper/abci.go similarity index 94% rename from x/staking/abci.go rename to x/staking/keeper/abci.go index 58dae0cea073..c0039ff21527 100644 --- a/x/staking/abci.go +++ b/x/staking/keeper/abci.go @@ -1,15 +1,14 @@ -package staking +package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/tendermint/tendermint/abci/types" ) // BeginBlocker will persist the current header and validator set as a historical entry // and prune the oldest entry based on the HistoricalEntries parameter -func BeginBlocker(ctx sdk.Context, k Keeper) { +func (k Keeper) BeginBlocker(ctx sdk.Context) { entryNum := k.HistoricalEntries(ctx) // Prune store to ensure we only have parameter-defined historical entries. @@ -42,7 +41,7 @@ func BeginBlocker(ctx sdk.Context, k Keeper) { } // Called every block, update validator set -func EndBlocker(ctx sdk.Context, k keeper.Keeper) []abci.ValidatorUpdate { +func (k Keeper) EndBlocker(ctx sdk.Context) []abci.ValidatorUpdate { // Calculate validator set changes. // // NOTE: ApplyAndReturnValidatorSetUpdates has to come before diff --git a/x/staking/abci_test.go b/x/staking/keeper/abci_test.go similarity index 80% rename from x/staking/abci_test.go rename to x/staking/keeper/abci_test.go index 3bb5847be9ab..426531d0cb47 100644 --- a/x/staking/abci_test.go +++ b/x/staking/keeper/abci_test.go @@ -1,4 +1,4 @@ -package staking +package keeper import ( "sort" @@ -7,13 +7,12 @@ import ( "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/tendermint/tendermint/abci/types" ) func TestBeginBlocker(t *testing.T) { - ctx, _, k, _ := keeper.CreateTestInput(t, false, 10) + ctx, _, k, _ := CreateTestInput(t, false, 10) // set historical entries in params to 5 params := types.DefaultParams() @@ -30,9 +29,9 @@ func TestBeginBlocker(t *testing.T) { ChainID: "HelloChain", Height: 5, } - valSet := []Validator{ - types.NewValidator(sdk.ValAddress(keeper.Addrs[0]), keeper.PKs[0], types.Description{}), - types.NewValidator(sdk.ValAddress(keeper.Addrs[1]), keeper.PKs[1], types.Description{}), + valSet := []types.Validator{ + types.NewValidator(sdk.ValAddress(Addrs[0]), PKs[0], types.Description{}), + types.NewValidator(sdk.ValAddress(Addrs[1]), PKs[1], types.Description{}), } hi4 := types.NewHistoricalInfo(h4, valSet) hi5 := types.NewHistoricalInfo(h5, valSet) @@ -46,10 +45,10 @@ func TestBeginBlocker(t *testing.T) { require.Equal(t, hi5, recv) // Set last validators in keeper - val1 := types.NewValidator(sdk.ValAddress(keeper.Addrs[2]), keeper.PKs[2], types.Description{}) + val1 := types.NewValidator(sdk.ValAddress(Addrs[2]), PKs[2], types.Description{}) k.SetValidator(ctx, val1) k.SetLastValidatorPower(ctx, val1.OperatorAddress, 10) - val2 := types.NewValidator(sdk.ValAddress(keeper.Addrs[3]), keeper.PKs[3], types.Description{}) + val2 := types.NewValidator(sdk.ValAddress(Addrs[3]), PKs[3], types.Description{}) vals := []types.Validator{val1, val2} sort.Sort(types.Validators(vals)) k.SetValidator(ctx, val2) @@ -62,7 +61,7 @@ func TestBeginBlocker(t *testing.T) { } ctx = ctx.WithBlockHeader(header) - BeginBlocker(ctx, k) + k.BeginBlocker(ctx) // Check HistoricalInfo at height 10 is persisted expected := types.HistoricalInfo{ diff --git a/x/staking/module.go b/x/staking/module.go index 69a8cb940504..12ccc954c5b2 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -166,13 +166,13 @@ func (am AppModule) ExportGenesis(ctx sdk.Context) json.RawMessage { // BeginBlock returns the begin blocker for the staking module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { - BeginBlocker(ctx, am.keeper) + am.keeper.BeginBlocker(ctx) } // EndBlock returns the end blocker for the staking module. It returns no validator // updates. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return EndBlocker(ctx, am.keeper) + return am.keeper.EndBlocker(ctx) } //____________________________________________________________________________ From 4402252d43aba88a312b01dbb185ed23b0b09539 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 17 Dec 2019 16:06:54 -0800 Subject: [PATCH 18/19] move beginblock/endblock logic into keeper, maintain API --- x/staking/abci.go | 18 +++++ x/staking/{keeper => }/abci_test.go | 15 ++-- x/staking/app_test.go | 2 +- x/staking/handler_test.go | 68 ++++++++--------- x/staking/keeper/abci.go | 96 ------------------------ x/staking/keeper/historical_info.go | 34 +++++++++ x/staking/keeper/historical_info_test.go | 72 ++++++++++++++++++ x/staking/keeper/val_state_change.go | 56 ++++++++++++++ x/staking/module.go | 4 +- 9 files changed, 225 insertions(+), 140 deletions(-) create mode 100644 x/staking/abci.go rename x/staking/{keeper => }/abci_test.go (81%) delete mode 100644 x/staking/keeper/abci.go diff --git a/x/staking/abci.go b/x/staking/abci.go new file mode 100644 index 000000000000..39755c24440c --- /dev/null +++ b/x/staking/abci.go @@ -0,0 +1,18 @@ +package staking + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/keeper" + abci "github.com/tendermint/tendermint/abci/types" +) + +// BeginBlocker will persist the current header and validator set as a historical entry +// and prune the oldest entry based on the HistoricalEntries parameter +func BeginBlocker(ctx sdk.Context, k keeper.Keeper) { + k.TrackHistoricalInfo(ctx) +} + +// Called every block, update validator set +func EndBlocker(ctx sdk.Context, k keeper.Keeper) []abci.ValidatorUpdate { + return k.BlockValidatorUpdates(ctx) +} diff --git a/x/staking/keeper/abci_test.go b/x/staking/abci_test.go similarity index 81% rename from x/staking/keeper/abci_test.go rename to x/staking/abci_test.go index 426531d0cb47..2a221621c408 100644 --- a/x/staking/keeper/abci_test.go +++ b/x/staking/abci_test.go @@ -1,4 +1,4 @@ -package keeper +package staking import ( "sort" @@ -7,12 +7,13 @@ import ( "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/staking/keeper" "github.com/cosmos/cosmos-sdk/x/staking/types" abci "github.com/tendermint/tendermint/abci/types" ) func TestBeginBlocker(t *testing.T) { - ctx, _, k, _ := CreateTestInput(t, false, 10) + ctx, _, k, _ := keeper.CreateTestInput(t, false, 10) // set historical entries in params to 5 params := types.DefaultParams() @@ -30,8 +31,8 @@ func TestBeginBlocker(t *testing.T) { Height: 5, } valSet := []types.Validator{ - types.NewValidator(sdk.ValAddress(Addrs[0]), PKs[0], types.Description{}), - types.NewValidator(sdk.ValAddress(Addrs[1]), PKs[1], types.Description{}), + types.NewValidator(sdk.ValAddress(keeper.Addrs[0]), keeper.PKs[0], types.Description{}), + types.NewValidator(sdk.ValAddress(keeper.Addrs[1]), keeper.PKs[1], types.Description{}), } hi4 := types.NewHistoricalInfo(h4, valSet) hi5 := types.NewHistoricalInfo(h5, valSet) @@ -45,10 +46,10 @@ func TestBeginBlocker(t *testing.T) { require.Equal(t, hi5, recv) // Set last validators in keeper - val1 := types.NewValidator(sdk.ValAddress(Addrs[2]), PKs[2], types.Description{}) + val1 := types.NewValidator(sdk.ValAddress(keeper.Addrs[2]), keeper.PKs[2], types.Description{}) k.SetValidator(ctx, val1) k.SetLastValidatorPower(ctx, val1.OperatorAddress, 10) - val2 := types.NewValidator(sdk.ValAddress(Addrs[3]), PKs[3], types.Description{}) + val2 := types.NewValidator(sdk.ValAddress(keeper.Addrs[3]), keeper.PKs[3], types.Description{}) vals := []types.Validator{val1, val2} sort.Sort(types.Validators(vals)) k.SetValidator(ctx, val2) @@ -61,7 +62,7 @@ func TestBeginBlocker(t *testing.T) { } ctx = ctx.WithBlockHeader(header) - k.BeginBlocker(ctx) + BeginBlocker(ctx, k) // Check HistoricalInfo at height 10 is persisted expected := types.HistoricalInfo{ diff --git a/x/staking/app_test.go b/x/staking/app_test.go index 911128fc49b1..0c2615d7a3ea 100644 --- a/x/staking/app_test.go +++ b/x/staking/app_test.go @@ -56,7 +56,7 @@ func getMockApp(t *testing.T) (*mock.App, Keeper) { // getEndBlocker returns a staking endblocker. func getEndBlocker(keeper Keeper) sdk.EndBlocker { return func(ctx sdk.Context, req abci.RequestEndBlock) abci.ResponseEndBlock { - validatorUpdates := keeper.EndBlocker(ctx) + validatorUpdates := EndBlocker(ctx, keeper) return abci.ResponseEndBlock{ ValidatorUpdates: validatorUpdates, diff --git a/x/staking/handler_test.go b/x/staking/handler_test.go index f06cb0b3bd3b..0e999ba9b43b 100644 --- a/x/staking/handler_test.go +++ b/x/staking/handler_test.go @@ -91,8 +91,8 @@ func TestValidatorByPowerIndex(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) + EndBlocker(ctx, keeper) // verify that by power key nolonger exists _, found = keeper.GetValidator(ctx, validatorAddr) @@ -216,7 +216,7 @@ func TestLegacyValidatorDelegations(t *testing.T) { var finishTime time.Time types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // verify the validator record still exists, is jailed, and has correct tokens validator, found = keeper.GetValidator(ctx, valAddr) @@ -438,7 +438,7 @@ func TestIncrementsMsgUnbond(t *testing.T) { var finishTime time.Time types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // check that the accounts and the bond account have the appropriate values validator, found = keeper.GetValidator(ctx, validatorAddr) @@ -532,7 +532,7 @@ func TestMultipleMsgCreateValidator(t *testing.T) { require.Equal(t, balanceExpd, balanceGot, "expected account to have %d, got %d", balanceExpd, balanceGot) } - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // unbond them all by removing delegation for i, validatorAddr := range validatorAddrs { @@ -548,10 +548,10 @@ func TestMultipleMsgCreateValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) // adds validator into unbonding queue - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // removes validator from queue and set - keeper.EndBlocker(ctx.WithBlockTime(blockTime.Add(params.UnbondingTime))) + EndBlocker(ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)), keeper) // Check that the validator is deleted from state validators := keeper.GetValidators(ctx, 100) @@ -599,7 +599,7 @@ func TestMultipleMsgDelegate(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // check that the account is unbonded _, found := keeper.GetDelegation(ctx, delegatorAddr, validatorAddr) @@ -631,7 +631,7 @@ func TestJailValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) validator, found := keeper.GetValidator(ctx, validatorAddr) require.True(t, found) @@ -645,7 +645,7 @@ func TestJailValidator(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // verify that the pubkey can now be reused got = handleMsgCreateValidator(ctx, msgCreateValidator, keeper) @@ -673,7 +673,7 @@ func TestValidatorQueue(t *testing.T) { got = handleMsgDelegate(ctx, msgDelegate, keeper) require.True(t, got.IsOK(), "expected ok, got %v", got) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // unbond the all self-delegation to put validator in unbonding state unbondAmt := sdk.NewCoin(sdk.DefaultBondDenom, delTokens) @@ -685,7 +685,7 @@ func TestValidatorQueue(t *testing.T) { types.ModuleCdc.MustUnmarshalBinaryLengthPrefixed(got.Data, &finishTime) ctx = ctx.WithBlockTime(finishTime) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) origHeader := ctx.BlockHeader() @@ -695,7 +695,7 @@ func TestValidatorQueue(t *testing.T) { // should still be unbonding at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) validator, found = keeper.GetValidator(ctx, validatorAddr) require.True(t, found) @@ -703,7 +703,7 @@ func TestValidatorQueue(t *testing.T) { // should be in unbonded state at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) validator, found = keeper.GetValidator(ctx, validatorAddr) require.True(t, found) @@ -725,7 +725,7 @@ func TestUnbondingPeriod(t *testing.T) { got := handleMsgCreateValidator(ctx, msgCreateValidator, keeper) require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // begin unbonding unbondAmt := sdk.NewCoin(sdk.DefaultBondDenom, sdk.TokensFromConsensusPower(10)) @@ -739,19 +739,19 @@ func TestUnbondingPeriod(t *testing.T) { require.True(t, found, "should not have unbonded") // cannot complete unbonding at same time - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.True(t, found, "should not have unbonded") // cannot complete unbonding at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.True(t, found, "should not have unbonded") // can complete unbonding at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) _, found = keeper.GetUnbondingDelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr) require.False(t, found, "should have unbonded") } @@ -789,7 +789,7 @@ func TestUnbondingFromUnbondingValidator(t *testing.T) { ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(keeper.UnbondingTime(ctx))) // Run the EndBlocker - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // Check to make sure that the unbonding delegation is no longer in state // (meaning it was deleted in the above EndBlocker) @@ -839,19 +839,19 @@ func TestRedelegationPeriod(t *testing.T) { origHeader := ctx.BlockHeader() // cannot complete redelegation at same time - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) _, found := keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.True(t, found, "should not have unbonded") // cannot complete redelegation at time 6 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 6)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) _, found = keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.True(t, found, "should not have unbonded") // can complete redelegation at time 7 seconds later ctx = ctx.WithBlockTime(origHeader.Time.Add(time.Second * 7)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) _, found = keeper.GetRedelegation(ctx, sdk.AccAddress(validatorAddr), validatorAddr, validatorAddr2) require.False(t, found, "should have unbonded") } @@ -893,7 +893,7 @@ func TestTransitiveRedelegation(t *testing.T) { ctx = ctx.WithBlockTime(blockTime.Add(params.UnbondingTime)) // complete first redelegation - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // now should be able to redelegate from the second validator to the third got = handleMsgBeginRedelegate(ctx, msgBeginRedelegate, keeper) @@ -921,7 +921,7 @@ func TestMultipleRedelegationAtSameTime(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond them - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // begin a redelegate selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -946,7 +946,7 @@ func TestMultipleRedelegationAtSameTime(t *testing.T) { // move forward in time, should complete both redelegations ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(1 * time.Second)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.False(t, found) @@ -973,7 +973,7 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond them - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // begin a redelegate selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -994,14 +994,14 @@ func TestMultipleRedelegationAtUniqueTimes(t *testing.T) { // move forward in time, should complete the first redelegation, but not the second ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.True(t, found) require.Len(t, rd.Entries, 1) // move forward in time, should complete the second redelegation ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) rd, found = keeper.GetRedelegation(ctx, selfDelAddr, valAddr, valAddr2) require.False(t, found) } @@ -1022,7 +1022,7 @@ func TestMultipleUnbondingDelegationAtSameTime(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // begin an unbonding delegation selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -1047,7 +1047,7 @@ func TestMultipleUnbondingDelegationAtSameTime(t *testing.T) { // move forwaubd in time, should complete both ubds ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(1 * time.Second)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.False(t, found) @@ -1069,7 +1069,7 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { require.True(t, got.IsOK(), "expected no error on runMsgCreateValidator") // end block to bond - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // begin an unbonding delegation selfDelAddr := sdk.AccAddress(valAddr) // (the validator is it's own delegator) @@ -1095,14 +1095,14 @@ func TestMultipleUnbondingDelegationAtUniqueTimes(t *testing.T) { // move forwaubd in time, should complete the first redelegation, but not the second ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.True(t, found) require.Len(t, ubd.Entries, 1) // move forwaubd in time, should complete the second redelegation ctx = ctx.WithBlockTime(ctx.BlockHeader().Time.Add(5 * time.Second)) - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) ubd, found = keeper.GetUnbondingDelegation(ctx, selfDelAddr, valAddr) require.False(t, found) } @@ -1254,7 +1254,7 @@ func TestBondUnbondRedelegateSlashTwice(t *testing.T) { require.Equal(t, sdk.NewDecFromInt(redAmt.Amount.QuoRaw(2)), delegation.Shares) // end blocker - keeper.EndBlocker(ctx) + EndBlocker(ctx, keeper) // validator power should have been reduced to zero // validator should be in unbonding state diff --git a/x/staking/keeper/abci.go b/x/staking/keeper/abci.go deleted file mode 100644 index c0039ff21527..000000000000 --- a/x/staking/keeper/abci.go +++ /dev/null @@ -1,96 +0,0 @@ -package keeper - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/staking/types" - abci "github.com/tendermint/tendermint/abci/types" -) - -// BeginBlocker will persist the current header and validator set as a historical entry -// and prune the oldest entry based on the HistoricalEntries parameter -func (k Keeper) BeginBlocker(ctx sdk.Context) { - entryNum := k.HistoricalEntries(ctx) - - // Prune store to ensure we only have parameter-defined historical entries. - // In most cases, this will involve removing a single historical entry. - // In the rare scenario when the historical entries gets reduced to a lower value k' - // from the original value k. k - k' entries must be deleted from the store. - // Since the entries to be deleted are always in a continuous range, we can iterate - // over the historical entries starting from the most recent version to be pruned - // and then return at the first empty entry. - for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- { - _, found := k.GetHistoricalInfo(ctx, i) - if found { - k.DeleteHistoricalInfo(ctx, i) - } else { - break - } - } - - // if there is no need to persist historicalInfo, return - if entryNum == 0 { - return - } - - // Create HistoricalInfo struct - lastVals := k.GetLastValidators(ctx) - historicalEntry := types.NewHistoricalInfo(ctx.BlockHeader(), lastVals) - - // Set latest HistoricalInfo at current height - k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) -} - -// Called every block, update validator set -func (k Keeper) EndBlocker(ctx sdk.Context) []abci.ValidatorUpdate { - // Calculate validator set changes. - // - // NOTE: ApplyAndReturnValidatorSetUpdates has to come before - // UnbondAllMatureValidatorQueue. - // This fixes a bug when the unbonding period is instant (is the case in - // some of the tests). The test expected the validator to be completely - // unbonded after the Endblocker (go from Bonded -> Unbonding during - // ApplyAndReturnValidatorSetUpdates and then Unbonding -> Unbonded during - // UnbondAllMatureValidatorQueue). - validatorUpdates := k.ApplyAndReturnValidatorSetUpdates(ctx) - - // Unbond all mature validators from the unbonding queue. - k.UnbondAllMatureValidatorQueue(ctx) - - // Remove all mature unbonding delegations from the ubd queue. - matureUnbonds := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time) - for _, dvPair := range matureUnbonds { - err := k.CompleteUnbonding(ctx, dvPair.DelegatorAddress, dvPair.ValidatorAddress) - if err != nil { - continue - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeCompleteUnbonding, - sdk.NewAttribute(types.AttributeKeyValidator, dvPair.ValidatorAddress.String()), - sdk.NewAttribute(types.AttributeKeyDelegator, dvPair.DelegatorAddress.String()), - ), - ) - } - - // Remove all mature redelegations from the red queue. - matureRedelegations := k.DequeueAllMatureRedelegationQueue(ctx, ctx.BlockHeader().Time) - for _, dvvTriplet := range matureRedelegations { - err := k.CompleteRedelegation(ctx, dvvTriplet.DelegatorAddress, - dvvTriplet.ValidatorSrcAddress, dvvTriplet.ValidatorDstAddress) - if err != nil { - continue - } - - ctx.EventManager().EmitEvent( - sdk.NewEvent( - types.EventTypeCompleteRedelegation, - sdk.NewAttribute(types.AttributeKeyDelegator, dvvTriplet.DelegatorAddress.String()), - sdk.NewAttribute(types.AttributeKeySrcValidator, dvvTriplet.ValidatorSrcAddress.String()), - sdk.NewAttribute(types.AttributeKeyDstValidator, dvvTriplet.ValidatorDstAddress.String()), - ), - ) - } - - return validatorUpdates -} diff --git a/x/staking/keeper/historical_info.go b/x/staking/keeper/historical_info.go index 5a71a78bce46..2ce0aee7dbcc 100644 --- a/x/staking/keeper/historical_info.go +++ b/x/staking/keeper/historical_info.go @@ -35,3 +35,37 @@ func (k Keeper) DeleteHistoricalInfo(ctx sdk.Context, height int64) { store.Delete(key) } + +// TrackHistoricalInfo saves the latest historical-info and deletes the oldest +// heights that are below pruning height +func (k Keeper) TrackHistoricalInfo(ctx sdk.Context) { + entryNum := k.HistoricalEntries(ctx) + + // Prune store to ensure we only have parameter-defined historical entries. + // In most cases, this will involve removing a single historical entry. + // In the rare scenario when the historical entries gets reduced to a lower value k' + // from the original value k. k - k' entries must be deleted from the store. + // Since the entries to be deleted are always in a continuous range, we can iterate + // over the historical entries starting from the most recent version to be pruned + // and then return at the first empty entry. + for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- { + _, found := k.GetHistoricalInfo(ctx, i) + if found { + k.DeleteHistoricalInfo(ctx, i) + } else { + break + } + } + + // if there is no need to persist historicalInfo, return + if entryNum == 0 { + return + } + + // Create HistoricalInfo struct + lastVals := k.GetLastValidators(ctx) + historicalEntry := types.NewHistoricalInfo(ctx.BlockHeader(), lastVals) + + // Set latest HistoricalInfo at current height + k.SetHistoricalInfo(ctx, ctx.BlockHeight(), historicalEntry) +} diff --git a/x/staking/keeper/historical_info_test.go b/x/staking/keeper/historical_info_test.go index ebdeea5370ec..fa1fa2356cd9 100644 --- a/x/staking/keeper/historical_info_test.go +++ b/x/staking/keeper/historical_info_test.go @@ -4,7 +4,9 @@ import ( "sort" "testing" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/staking/types" + abci "github.com/tendermint/tendermint/abci/types" "github.com/stretchr/testify/require" ) @@ -32,3 +34,73 @@ func TestHistoricalInfo(t *testing.T) { require.False(t, found, "HistoricalInfo found after delete") require.Equal(t, types.HistoricalInfo{}, recv, "HistoricalInfo is not empty") } + +func TestTrackHistoricalInfo(t *testing.T) { + ctx, _, k, _ := CreateTestInput(t, false, 10) + + // set historical entries in params to 5 + params := types.DefaultParams() + params.HistoricalEntries = 5 + k.SetParams(ctx, params) + + // set historical info at 5, 4 which should be pruned + // and check that it has been stored + h4 := abci.Header{ + ChainID: "HelloChain", + Height: 4, + } + h5 := abci.Header{ + ChainID: "HelloChain", + Height: 5, + } + valSet := []types.Validator{ + types.NewValidator(sdk.ValAddress(Addrs[0]), PKs[0], types.Description{}), + types.NewValidator(sdk.ValAddress(Addrs[1]), PKs[1], types.Description{}), + } + hi4 := types.NewHistoricalInfo(h4, valSet) + hi5 := types.NewHistoricalInfo(h5, valSet) + k.SetHistoricalInfo(ctx, 4, hi4) + k.SetHistoricalInfo(ctx, 5, hi5) + recv, found := k.GetHistoricalInfo(ctx, 4) + require.True(t, found) + require.Equal(t, hi4, recv) + recv, found = k.GetHistoricalInfo(ctx, 5) + require.True(t, found) + require.Equal(t, hi5, recv) + + // Set last validators in keeper + val1 := types.NewValidator(sdk.ValAddress(Addrs[2]), PKs[2], types.Description{}) + k.SetValidator(ctx, val1) + k.SetLastValidatorPower(ctx, val1.OperatorAddress, 10) + val2 := types.NewValidator(sdk.ValAddress(Addrs[3]), PKs[3], types.Description{}) + vals := []types.Validator{val1, val2} + sort.Sort(types.Validators(vals)) + k.SetValidator(ctx, val2) + k.SetLastValidatorPower(ctx, val2.OperatorAddress, 8) + + // Set Header for BeginBlock context + header := abci.Header{ + ChainID: "HelloChain", + Height: 10, + } + ctx = ctx.WithBlockHeader(header) + + k.TrackHistoricalInfo(ctx) + + // Check HistoricalInfo at height 10 is persisted + expected := types.HistoricalInfo{ + Header: header, + ValSet: vals, + } + recv, found = k.GetHistoricalInfo(ctx, 10) + require.True(t, found, "GetHistoricalInfo failed after BeginBlock") + require.Equal(t, expected, recv, "GetHistoricalInfo returned eunexpected result") + + // Check HistoricalInfo at height 5, 4 is pruned + recv, found = k.GetHistoricalInfo(ctx, 4) + require.False(t, found, "GetHistoricalInfo did not prune earlier height") + require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo at height 4 is not empty after prune") + recv, found = k.GetHistoricalInfo(ctx, 5) + require.False(t, found, "GetHistoricalInfo did not prune first prune height") + require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo at height 5 is not empty after prune") +} diff --git a/x/staking/keeper/val_state_change.go b/x/staking/keeper/val_state_change.go index 724ebaff0590..f43a8f9da5a7 100644 --- a/x/staking/keeper/val_state_change.go +++ b/x/staking/keeper/val_state_change.go @@ -11,6 +11,62 @@ import ( "github.com/cosmos/cosmos-sdk/x/staking/types" ) +// Calculate the ValidatorUpdates for the current block +// Called in each EndBlock +func (k Keeper) BlockValidatorUpdates(ctx sdk.Context) []abci.ValidatorUpdate { + // Calculate validator set changes. + // + // NOTE: ApplyAndReturnValidatorSetUpdates has to come before + // UnbondAllMatureValidatorQueue. + // This fixes a bug when the unbonding period is instant (is the case in + // some of the tests). The test expected the validator to be completely + // unbonded after the Endblocker (go from Bonded -> Unbonding during + // ApplyAndReturnValidatorSetUpdates and then Unbonding -> Unbonded during + // UnbondAllMatureValidatorQueue). + validatorUpdates := k.ApplyAndReturnValidatorSetUpdates(ctx) + + // Unbond all mature validators from the unbonding queue. + k.UnbondAllMatureValidatorQueue(ctx) + + // Remove all mature unbonding delegations from the ubd queue. + matureUnbonds := k.DequeueAllMatureUBDQueue(ctx, ctx.BlockHeader().Time) + for _, dvPair := range matureUnbonds { + err := k.CompleteUnbonding(ctx, dvPair.DelegatorAddress, dvPair.ValidatorAddress) + if err != nil { + continue + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeCompleteUnbonding, + sdk.NewAttribute(types.AttributeKeyValidator, dvPair.ValidatorAddress.String()), + sdk.NewAttribute(types.AttributeKeyDelegator, dvPair.DelegatorAddress.String()), + ), + ) + } + + // Remove all mature redelegations from the red queue. + matureRedelegations := k.DequeueAllMatureRedelegationQueue(ctx, ctx.BlockHeader().Time) + for _, dvvTriplet := range matureRedelegations { + err := k.CompleteRedelegation(ctx, dvvTriplet.DelegatorAddress, + dvvTriplet.ValidatorSrcAddress, dvvTriplet.ValidatorDstAddress) + if err != nil { + continue + } + + ctx.EventManager().EmitEvent( + sdk.NewEvent( + types.EventTypeCompleteRedelegation, + sdk.NewAttribute(types.AttributeKeyDelegator, dvvTriplet.DelegatorAddress.String()), + sdk.NewAttribute(types.AttributeKeySrcValidator, dvvTriplet.ValidatorSrcAddress.String()), + sdk.NewAttribute(types.AttributeKeyDstValidator, dvvTriplet.ValidatorDstAddress.String()), + ), + ) + } + + return validatorUpdates +} + // Apply and return accumulated updates to the bonded validator set. Also, // * Updates the active valset as keyed by LastValidatorPowerKey. // * Updates the total power as keyed by LastTotalPowerKey. diff --git a/x/staking/module.go b/x/staking/module.go index 12ccc954c5b2..69a8cb940504 100644 --- a/x/staking/module.go +++ b/x/staking/module.go @@ -166,13 +166,13 @@ func (am AppModule) ExportGenesis(ctx sdk.Context) json.RawMessage { // BeginBlock returns the begin blocker for the staking module. func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) { - am.keeper.BeginBlocker(ctx) + BeginBlocker(ctx, am.keeper) } // EndBlock returns the end blocker for the staking module. It returns no validator // updates. func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { - return am.keeper.EndBlocker(ctx) + return EndBlocker(ctx, am.keeper) } //____________________________________________________________________________ From c3ad7934b4ba1025aa789961c859f895508e836c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 17 Dec 2019 16:07:19 -0800 Subject: [PATCH 19/19] remove redundant abci tests --- x/staking/abci_test.go | 83 ------------------------------------------ 1 file changed, 83 deletions(-) delete mode 100644 x/staking/abci_test.go diff --git a/x/staking/abci_test.go b/x/staking/abci_test.go deleted file mode 100644 index 2a221621c408..000000000000 --- a/x/staking/abci_test.go +++ /dev/null @@ -1,83 +0,0 @@ -package staking - -import ( - "sort" - "testing" - - "github.com/stretchr/testify/require" - - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/staking/keeper" - "github.com/cosmos/cosmos-sdk/x/staking/types" - abci "github.com/tendermint/tendermint/abci/types" -) - -func TestBeginBlocker(t *testing.T) { - ctx, _, k, _ := keeper.CreateTestInput(t, false, 10) - - // set historical entries in params to 5 - params := types.DefaultParams() - params.HistoricalEntries = 5 - k.SetParams(ctx, params) - - // set historical info at 5, 4 which should be pruned - // and check that it has been stored - h4 := abci.Header{ - ChainID: "HelloChain", - Height: 4, - } - h5 := abci.Header{ - ChainID: "HelloChain", - Height: 5, - } - valSet := []types.Validator{ - types.NewValidator(sdk.ValAddress(keeper.Addrs[0]), keeper.PKs[0], types.Description{}), - types.NewValidator(sdk.ValAddress(keeper.Addrs[1]), keeper.PKs[1], types.Description{}), - } - hi4 := types.NewHistoricalInfo(h4, valSet) - hi5 := types.NewHistoricalInfo(h5, valSet) - k.SetHistoricalInfo(ctx, 4, hi4) - k.SetHistoricalInfo(ctx, 5, hi5) - recv, found := k.GetHistoricalInfo(ctx, 4) - require.True(t, found) - require.Equal(t, hi4, recv) - recv, found = k.GetHistoricalInfo(ctx, 5) - require.True(t, found) - require.Equal(t, hi5, recv) - - // Set last validators in keeper - val1 := types.NewValidator(sdk.ValAddress(keeper.Addrs[2]), keeper.PKs[2], types.Description{}) - k.SetValidator(ctx, val1) - k.SetLastValidatorPower(ctx, val1.OperatorAddress, 10) - val2 := types.NewValidator(sdk.ValAddress(keeper.Addrs[3]), keeper.PKs[3], types.Description{}) - vals := []types.Validator{val1, val2} - sort.Sort(types.Validators(vals)) - k.SetValidator(ctx, val2) - k.SetLastValidatorPower(ctx, val2.OperatorAddress, 8) - - // Set Header for BeginBlock context - header := abci.Header{ - ChainID: "HelloChain", - Height: 10, - } - ctx = ctx.WithBlockHeader(header) - - BeginBlocker(ctx, k) - - // Check HistoricalInfo at height 10 is persisted - expected := types.HistoricalInfo{ - Header: header, - ValSet: vals, - } - recv, found = k.GetHistoricalInfo(ctx, 10) - require.True(t, found, "GetHistoricalInfo failed after BeginBlock") - require.Equal(t, expected, recv, "GetHistoricalInfo returned eunexpected result") - - // Check HistoricalInfo at height 5, 4 is pruned - recv, found = k.GetHistoricalInfo(ctx, 4) - require.False(t, found, "GetHistoricalInfo did not prune earlier height") - require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo at height 4 is not empty after prune") - recv, found = k.GetHistoricalInfo(ctx, 5) - require.False(t, found, "GetHistoricalInfo did not prune first prune height") - require.Equal(t, types.HistoricalInfo{}, recv, "GetHistoricalInfo at height 5 is not empty after prune") -}