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 1 commit
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
Prev Previous commit
Next Next commit
docs and CHANGELOG
  • Loading branch information
AdityaSripal committed Dec 10, 2019
commit da266e8a8157531ade28dd2d070a5bca4ae8a790
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
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.


### Improvements

Expand Down
3 changes: 2 additions & 1 deletion x/staking/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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
Expand Down
3 changes: 3 additions & 0 deletions x/staking/keeper/historical_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
store := ctx.KVStore(k.storeKey)
key := types.GetHistoricalInfoKey(height)
Expand All @@ -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)
Expand All @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions x/staking/types/historical_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
ValSet []Validator
}

//NewHistoricalInfo will create a historical information struct from header and valset
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// it will first sort valset before inclusion into historical info
func NewHistoricalInfo(header abci.Header, valSet []Validator) HistoricalInfo {
sort.Sort(Validators(valSet))
return HistoricalInfo{
Expand All @@ -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 {
Expand All @@ -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 {
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
if hi.ValSet != nil {
return sdkerrors.Wrap(ErrInvalidHistoricalInfo(DefaultCodespace), "ValidatorSer is nil")
Expand Down
2 changes: 1 addition & 1 deletion x/staking/types/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
}