Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ADR 17 Implementation: Historical Module #5380

Merged
merged 31 commits into from
Dec 18, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
17fcad3
add history module and some tests
AdityaSripal Dec 10, 2019
5f916a4
Merge branch 'aditya/history' of https://github.com/cosmos/cosmos-sdk…
AdityaSripal Dec 10, 2019
2b052b4
complete query and write tests
AdityaSripal Dec 10, 2019
481ee0d
complete query and write tests
AdityaSripal Dec 10, 2019
954c8b1
add abci test
AdityaSripal Dec 10, 2019
da266e8
docs and CHANGELOG
AdityaSripal Dec 10, 2019
813c634
add client query methods and CHANGELOG
AdityaSripal Dec 11, 2019
5c634a3
fix merge
AdityaSripal Dec 11, 2019
33154f0
Merge branch 'master' into aditya/history
cwgoes Dec 11, 2019
aa34dc7
implement edge case for when historicalentries param gets reduced
AdityaSripal Dec 11, 2019
fb2e660
Merge branch 'aditya/history' of https://github.com/cosmos/cosmos-sdk…
AdityaSripal Dec 11, 2019
d0e8d88
Update x/staking/abci.go
cwgoes Dec 11, 2019
5997e15
Apply suggestions from code review
AdityaSripal Dec 12, 2019
82d386f
apply codereview suggestions
AdityaSripal Dec 12, 2019
a45a5fa
appease linter
AdityaSripal Dec 13, 2019
ef43df5
Update x/staking/client/cli/query.go
fedekunze Dec 16, 2019
093dcf5
Merge branch 'master' into aditya/history
fedekunze Dec 16, 2019
c3fc30b
Apply suggestions from code review
AdityaSripal Dec 16, 2019
e4d2c34
Update x/staking/keeper/historical_info.go
AdityaSripal Dec 16, 2019
74b3f19
update spec, alias, and fix minor bug
AdityaSripal Dec 17, 2019
73cbb7e
cleanup changelog
AdityaSripal Dec 17, 2019
277054b
Merge branch 'master' into aditya/history
AdityaSripal Dec 17, 2019
a61d28a
Merge branch 'master' of https://github.com/cosmos/cosmos-sdk into ad…
AdityaSripal Dec 17, 2019
1fdc269
Merge branch 'aditya/history' of https://github.com/cosmos/cosmos-sdk…
AdityaSripal Dec 17, 2019
3f59d8c
fix merge
AdityaSripal Dec 17, 2019
9364ba4
Make BeginBlocker/EndBlocker methods on keeper
AdityaSripal Dec 17, 2019
c0f2ac7
Merge branch 'master' into aditya/history
alexanderbez Dec 17, 2019
4402252
move beginblock/endblock logic into keeper, maintain API
AdityaSripal Dec 18, 2019
c3ad793
remove redundant abci tests
AdityaSripal Dec 18, 2019
b1a9ac5
Merge branch 'aditya/history' of https://github.com/cosmos/cosmos-sdk…
AdityaSripal Dec 18, 2019
ffc64bf
Merge branch 'master' into aditya/history
AdityaSripal Dec 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,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
Copy link
Contributor

Choose a reason for hiding this comment

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

A single changelog entry should be made with bullet points if need be.

* (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
* (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).

### Improvements
Expand Down
43 changes: 43 additions & 0 deletions x/staking/abci.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package staking

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/staking/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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Best to move this entire block into a method on the keeper.

  1. This keeps the BeginBlocker abstraction clean.
  2. Improves testability.

Copy link
Member Author

Choose a reason for hiding this comment

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

Open to doing this here, but what i have follows the pattern of BeginBlockers in x/slashing, x/supply, and x/upgrade

Copy link
Contributor

Choose a reason for hiding this comment

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

They should be updated as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about the EndBlocker? Since that isn't a method on the keeper either?

// 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{
Copy link
Collaborator

Choose a reason for hiding this comment

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

we can use the NewHistoricalInfo constructor here

Copy link
Collaborator

Choose a reason for hiding this comment

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

also, the validation (+ panic on error) is missing

Copy link
Member Author

Choose a reason for hiding this comment

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

Don't know if validation here is necessary. Validation is true by construction, so long as Tendermint consensus is working correctly

Copy link
Collaborator

Choose a reason for hiding this comment

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

where is the validation used then?

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
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// in most cases, this will invole removing a single historical entry
cwgoes marked this conversation as resolved.
Show resolved Hide resolved
// 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
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// 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
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
_, found := k.GetHistoricalInfo(ctx, i)
if found {
k.DeleteHistoricalInfo(ctx, i)
} else {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
return
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
}
}
83 changes: 83 additions & 0 deletions x/staking/abci_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
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 := []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")
}
47 changes: 47 additions & 0 deletions x/staking/client/cli/query.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package cli

import (
"errors"
"fmt"
"strconv"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -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))...)

Expand Down Expand Up @@ -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))
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}

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{
Expand Down
37 changes: 37 additions & 0 deletions x/staking/client/rest/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package rest
import (
"fmt"
"net/http"
"strconv"
"strings"

"github.com/gorilla/mux"
Expand Down Expand Up @@ -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}",
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
historicalInfoHandlerFn(cliCtx),
).Methods("GET")

// Get the current state of the staking pool
r.HandleFunc(
"/staking/pool",
Expand Down Expand Up @@ -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.Sprintf("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) {
Expand Down
38 changes: 38 additions & 0 deletions x/staking/keeper/historical_info.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"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) {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
store := ctx.KVStore(k.storeKey)
key := types.GetHistoricalInfoKey(height)

value := store.Get(key)

AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
if value == nil {
return types.HistoricalInfo{}, false
}

hi = types.MustUnmarshalHistoricalInfo(k.cdc, value)
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)

value := types.MustMarshalHistoricalInfo(k.cdc, hi)
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)

store.Delete(key)
}
34 changes: 34 additions & 0 deletions x/staking/keeper/historical_info_test.go
Original file line number Diff line number Diff line change
@@ -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
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved

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")
}
8 changes: 8 additions & 0 deletions x/staking/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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),
)
}
Expand Down
23 changes: 23 additions & 0 deletions x/staking/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -327,6 +329,27 @@ 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) {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
var params types.QueryHistoricalInfoParams

err := types.ModuleCdc.UnmarshalJSON(req.Data, &params)
if err != nil {
return nil, sdk.ErrUnknownRequest(string(req.Data))
}

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) {
bondDenom := k.BondDenom(ctx)
bondedPool := k.GetBondedPool(ctx)
Expand Down
Loading