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

Staking store keys bug fix + memory improvement #2435

Merged
merged 2 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ BREAKING CHANGES
* `cosmosaccaddr` / `cosmosaccpub` => `cosmos` / `cosmospub`
* `cosmosvaladdr` / `cosmosvalpub` => `cosmosvaloper` / `cosmosvaloperpub`
* [x/stake] [#1013] TendermintUpdates now uses transient store
* [x/stake] \#2435 Remove empty bytes from the ValidatorPowerRank store key
* [x/gov] [#2195] Governance uses BFT Time
* [x/gov] \#2256 Removed slashing for governance non-voting validators
* [simulation] \#2162 Added back correct supply invariants
Expand Down Expand Up @@ -129,6 +130,7 @@ IMPROVEMENTS
* [x/auth] Signature verification's gas cost now accounts for pubkey type. [#2046](https://github.com/tendermint/tendermint/pull/2046)
* [x/stake] [x/slashing] Ensure delegation invariants to jailed validators [#1883](https://github.com/cosmos/cosmos-sdk/issues/1883).
* [x/stake] Improve speed of GetValidator, which was shown to be a performance bottleneck. [#2046](https://github.com/tendermint/tendermint/pull/2200)
* [x/stake] \#2435 Improve memory efficiency of getting the various store keys
* [genesis] \#2229 Ensure that there are no duplicate accounts or validators in the genesis state.
* Add SDK validation to `config.toml` (namely disabling `create_empty_blocks`) \#1571
* \#1941(https://github.com/cosmos/cosmos-sdk/issues/1941) Version is now inferred via `git describe --tags`.
Expand Down
63 changes: 39 additions & 24 deletions x/stake/keeper/key.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func GetAddressFromValBondedIndexKey(IndexKey []byte) []byte {
// VALUE: validator operator address ([]byte)
func GetValidatorsByPowerIndexKey(validator types.Validator, pool types.Pool) []byte {
// NOTE the address doesn't need to be stored because counter bytes must always be different
return getValidatorPowerRank(validator, pool)
return getValidatorPowerRank(validator)
}

// get the bonded validator index key for an operator address
Expand All @@ -63,22 +63,23 @@ func GetBondedValidatorIndexKey(operator sdk.ValAddress) []byte {
// get the power ranking of a validator
// NOTE the larger values are of higher value
// nolint: unparam
func getValidatorPowerRank(validator types.Validator, pool types.Pool) []byte {
func getValidatorPowerRank(validator types.Validator) []byte {

potentialPower := validator.Tokens
powerBytes := []byte(potentialPower.ToLeftPadded(maxDigitsForAccount)) // power big-endian (more powerful validators first)
powerBytesLen := len(powerBytes)
// key is of format prefix || powerbytes || heightBytes || counterBytes
key := make([]byte, 1+powerBytesLen+8+2)

// heightBytes and counterBytes represent strings like powerBytes does
heightBytes := make([]byte, binary.MaxVarintLen64)
binary.BigEndian.PutUint64(heightBytes, ^uint64(validator.BondHeight)) // invert height (older validators first)
counterBytes := make([]byte, 2)
binary.BigEndian.PutUint16(counterBytes, ^uint16(validator.BondIntraTxCounter)) // invert counter (first txns have priority)
key[0] = ValidatorsByPowerIndexKey[0]
copy(key[1:powerBytesLen+1], powerBytes)

return append(append(append(
ValidatorsByPowerIndexKey,
powerBytes...),
heightBytes...),
counterBytes...)
// include heightBytes height is inverted (older validators first)
binary.BigEndian.PutUint64(key[powerBytesLen+1:powerBytesLen+9], ^uint64(validator.BondHeight))
// include counterBytes, counter is inverted (first txns have priority)
binary.BigEndian.PutUint16(key[powerBytesLen+9:powerBytesLen+11], ^uint16(validator.BondIntraTxCounter))

return key
}

//______________________________________________________________________________
Expand Down Expand Up @@ -138,28 +139,42 @@ func GetUBDsByValIndexKey(valAddr sdk.ValAddress) []byte {
// gets the key for a redelegation
// VALUE: stake/types.RedelegationKey
func GetREDKey(delAddr sdk.AccAddress, valSrcAddr, valDstAddr sdk.ValAddress) []byte {
return append(append(
GetREDsKey(delAddr.Bytes()),
valSrcAddr.Bytes()...),
valDstAddr.Bytes()...)
key := make([]byte, 1+sdk.AddrLen*3)

copy(key[0:sdk.AddrLen+1], GetREDsKey(delAddr.Bytes()))
copy(key[sdk.AddrLen+1:2*sdk.AddrLen+1], valSrcAddr.Bytes())
copy(key[2*sdk.AddrLen+1:3*sdk.AddrLen+1], valDstAddr.Bytes())

return key
}

// gets the index-key for a redelegation, stored by source-validator-index
// VALUE: none (key rearrangement used)
func GetREDByValSrcIndexKey(delAddr sdk.AccAddress, valSrcAddr, valDstAddr sdk.ValAddress) []byte {
return append(append(
GetREDsFromValSrcIndexKey(valSrcAddr),
delAddr.Bytes()...),
valDstAddr.Bytes()...)
REDSFromValsSrcKey := GetREDsFromValSrcIndexKey(valSrcAddr)
offset := len(REDSFromValsSrcKey)

// key is of the form REDSFromValsSrcKey || delAddr || valDstAddr
key := make([]byte, len(REDSFromValsSrcKey)+2*sdk.AddrLen)
copy(key[0:offset], REDSFromValsSrcKey)
copy(key[offset:offset+sdk.AddrLen], delAddr.Bytes())
copy(key[offset+sdk.AddrLen:offset+2*sdk.AddrLen], valDstAddr.Bytes())
return key
}

// gets the index-key for a redelegation, stored by destination-validator-index
// VALUE: none (key rearrangement used)
func GetREDByValDstIndexKey(delAddr sdk.AccAddress, valSrcAddr, valDstAddr sdk.ValAddress) []byte {
return append(append(
GetREDsToValDstIndexKey(valDstAddr),
delAddr.Bytes()...),
valSrcAddr.Bytes()...)
REDSToValsDstKey := GetREDsToValDstIndexKey(valDstAddr)
offset := len(REDSToValsDstKey)

// key is of the form REDSToValsDstKey || delAddr || valSrcAddr
key := make([]byte, len(REDSToValsDstKey)+2*sdk.AddrLen)
copy(key[0:offset], REDSToValsDstKey)
copy(key[offset:offset+sdk.AddrLen], delAddr.Bytes())
copy(key[offset+sdk.AddrLen:offset+2*sdk.AddrLen], valSrcAddr.Bytes())

return key
}

// rearranges the ValSrcIndexKey to get the REDKey
Expand Down
35 changes: 35 additions & 0 deletions x/stake/keeper/key_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package keeper

import (
"encoding/hex"
"testing"

"github.com/stretchr/testify/assert"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/stake/types"
"github.com/tendermint/tendermint/crypto/ed25519"
)

var (
pk1 = ed25519.GenPrivKey().PubKey()
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have unit tests for GetREDByValSrcIndexKey and GetREDByValDstIndexKey? Otherwise, this LGTM. Thanks 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have any to my knowledge. I can make some

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do that, but it can be in a separate PR.

func TestGetValidatorPowerRank(t *testing.T) {
valAddr1 := sdk.ValAddress(pk1.Bytes())
emptyDesc := types.Description{}
val1 := types.NewValidator(valAddr1, pk1, emptyDesc)
val1.Tokens = sdk.NewDec(12)

tests := []struct {
validator types.Validator
wantHex string
}{
{val1, "05303030303030303030303132ffffffffffffffffffff"},
}
for i, tt := range tests {
got := hex.EncodeToString(getValidatorPowerRank(tt.validator))

assert.Equal(t, tt.wantHex, got, "Keys did not match on test case %d", i)
}
}