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.

Sign up for GitHub

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

R4R: Fix Invalid Cliff Validator Power Comparison #2077

Merged
merged 3 commits into from
Aug 17, 2018
Merged

R4R: Fix Invalid Cliff Validator Power Comparison #2077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac84f20
Fix invalid power check updateCliffValidator and add defensive panic
Aug 17, 2018
9864209
Update PENDING log
Aug 17, 2018
a2cca2c
Pending -> Changelog
cwgoes Aug 17, 2018
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ BUG FIXES
- [x/stake] \#2021 Fixed repeated CLI commands in staking

* Gaia
- [x/stake] [#2077](https://github.com/cosmos/cosmos-sdk/pull/2077) Fixed invalid cliff power comparison
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't need to put the links in the pending file I don't think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok -- I'll stop doing that.

- \#1804 Fixes gen-tx genesis generation logic temporarily until upstream updates
- \#1799 Fix `gaiad export`
- \#1839 Fixed bug where intra-tx counter wasn't set correctly for genesis validators
Expand Down
19 changes: 9 additions & 10 deletions x/stake/keeper/validator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,6 @@ func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validato
panic(fmt.Sprintf("cliff validator record not found for address: %v\n", cliffAddr))
}

// NOTE: We get the power via affectedVal since the store (by power key)
// has yet to be updated.
affectedValPower := affectedVal.GetPower()

// Create a validator iterator ranging from smallest to largest by power
// starting the current cliff validator's power.
start := GetValidatorsByPowerIndexKey(oldCliffVal, pool)
Expand All @@ -307,16 +303,19 @@ func (k Keeper) updateCliffValidator(ctx sdk.Context, affectedVal types.Validato
panic("failed to create valid validator power iterator")
}

affectedValRank := GetValidatorsByPowerIndexKey(affectedVal, pool)
newCliffValRank := GetValidatorsByPowerIndexKey(newCliffVal, pool)

if bytes.Equal(affectedVal.Owner, newCliffVal.Owner) {
// The affected validator remains the cliff validator, however, since
// the store does not contain the new power, set the new cliff
// validator to the affected validator.
bz := GetValidatorsByPowerIndexKey(affectedVal, pool)
store.Set(ValidatorPowerCliffKey, bz)
} else if affectedValPower.GT(newCliffVal.GetPower()) {
// the store does not contain the new power, update the new power rank.
store.Set(ValidatorPowerCliffKey, affectedValRank)
} else if bytes.Compare(affectedValRank, newCliffValRank) > 0 {
// The affected validator no longer remains the cliff validator as it's
// power is greater than the new current cliff validator.
// power is greater than the new cliff validator.
k.setCliffValidator(ctx, newCliffVal, pool)
} else {
panic("invariant broken: the cliff validator should change or it should remain the same")
}
}

Expand Down
14 changes: 13 additions & 1 deletion x/stake/keeper/validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ func TestSetValidator(t *testing.T) {
updates := keeper.GetTendermintUpdates(ctx)
require.Equal(t, 1, len(updates))
require.Equal(t, validator.ABCIValidator(), updates[0])

}

func TestUpdateValidatorByPowerIndex(t *testing.T) {
Expand Down Expand Up @@ -143,6 +142,19 @@ func TestCliffValidatorChange(t *testing.T) {
cliffPower = keeper.GetCliffValidatorPower(ctx)
require.Equal(t, newCliffVal.Owner, sdk.AccAddress(keeper.GetCliffValidator(ctx)))
require.Equal(t, GetValidatorsByPowerIndexKey(newCliffVal, pool), cliffPower)

// add enough power to cliff validator to be equal in rank to next validator
newCliffVal, pool, _ = newCliffVal.AddTokensFromDel(pool, 9)
keeper.SetPool(ctx, pool)
newCliffVal = keeper.UpdateValidator(ctx, newCliffVal)

// assert new cliff validator due to power rank construction
newCliffVal = validators[numVals-maxVals+2]
require.Equal(t, newCliffVal.Owner, sdk.AccAddress(keeper.GetCliffValidator(ctx)))

// assert cliff validator power should have been updated
cliffPower = keeper.GetCliffValidatorPower(ctx)
require.Equal(t, GetValidatorsByPowerIndexKey(newCliffVal, pool), cliffPower)
}

func TestSlashToZeroPowerRemoved(t *testing.T) {
Expand Down