-
Notifications
You must be signed in to change notification settings - Fork 202
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
fix(oracle)!: count vote omission as abstain for less slashing + more stability #1565
Changes from 16 commits
2b8cd27
6ad181b
765f5a3
c490ffc
02a2e9d
9cbae1f
a53b360
9adc9e4
c6a592d
3ca259a
b9862f4
d4e9bb8
70001e9
f741d7f
a0cf656
0a9be17
fad86ca
f8bba05
f749e9a
1ee6408
bff235a
968958d
d1467ff
32c34cb
51b18cc
1b48b90
af92024
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,22 +105,22 @@ func TestInvalidVotesSlashing(t *testing.T) { | |
|
||
// Account 1, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}, | ||
}, 0) | ||
|
||
// Account 2, govstable, miss vote | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate.Add(sdk.NewDec(100000000000000))}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE.Add(sdk.NewDec(100000000000000))}, | ||
}, 1) | ||
|
||
// Account 3, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}, | ||
}, 2) | ||
|
||
// Account 4, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}, | ||
}, 3) | ||
|
||
input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
|
@@ -131,27 +131,27 @@ func TestInvalidVotesSlashing(t *testing.T) { | |
} | ||
|
||
validator := input.StakingKeeper.Validator(input.Ctx, ValAddrs[1]) | ||
require.Equal(t, stakingAmt, validator.GetBondedTokens()) | ||
require.Equal(t, TEST_STAKING_AMT, validator.GetBondedTokens()) | ||
|
||
// one more miss vote will inccur ValAddrs[1] slashing | ||
// Account 1, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}, | ||
}, 0) | ||
|
||
// Account 2, govstable, miss vote | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate.Add(sdk.NewDec(100000000000000))}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE.Add(sdk.NewDec(100000000000000))}, | ||
}, 1) | ||
|
||
// Account 3, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}, | ||
}, 2) | ||
|
||
// Account 4, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{ | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}, | ||
{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}, | ||
}, 3) | ||
|
||
input.Ctx = input.Ctx.WithBlockHeight(votePeriodsPerWindow - 1) | ||
|
@@ -160,46 +160,72 @@ func TestInvalidVotesSlashing(t *testing.T) { | |
// input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
|
||
validator = input.StakingKeeper.Validator(input.Ctx, ValAddrs[1]) | ||
require.Equal(t, sdk.OneDec().Sub(slashFraction).MulInt(stakingAmt).TruncateInt(), validator.GetBondedTokens()) | ||
require.Equal(t, sdk.OneDec().Sub(slashFraction).MulInt(TEST_STAKING_AMT).TruncateInt(), validator.GetBondedTokens()) | ||
} | ||
|
||
// TestWhitelistSlashing: Creates a scenario where one valoper (valIdx 0) does | ||
// not vote throughout an entire vote window, while valopers 1 and 2 do. | ||
func TestWhitelistSlashing(t *testing.T) { | ||
input, h := Setup(t) | ||
input, msgServer := Setup(t) | ||
|
||
votePeriodsPerWindow := sdk.NewDec(int64(input.OracleKeeper.SlashWindow(input.Ctx))).QuoInt64(int64(input.OracleKeeper.VotePeriod(input.Ctx))).TruncateInt64() | ||
slashFraction := input.OracleKeeper.SlashFraction(input.Ctx) | ||
// slashFraction := input.OracleKeeper.SlashFraction(input.Ctx) | ||
minValidPerWindow := input.OracleKeeper.MinValidPerWindow(input.Ctx) | ||
|
||
for i := uint64(0); i < uint64(sdk.OneDec().Sub(minValidPerWindow).MulInt64(votePeriodsPerWindow).TruncateInt64()); i++ { | ||
input.Ctx = input.Ctx.WithBlockHeight(input.Ctx.BlockHeight() + 1) | ||
pair := asset.Registry.Pair(denoms.NIBI, denoms.NUSD) | ||
priceVoteFromVal := func(valIdx int, block int64, erate sdk.Dec) { | ||
MakeAggregatePrevoteAndVote(t, input, msgServer, block, | ||
types.ExchangeRateTuples{{Pair: pair, ExchangeRate: erate}}, | ||
valIdx) | ||
} | ||
input.OracleKeeper.WhitelistedPairs.Insert(input.Ctx, pair) | ||
perfs := input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
require.EqualValues(t, 0, perfs.GetTotalRewardWeight()) | ||
|
||
// Account 2, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 1) | ||
// Account 3, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 2) | ||
block := input.Ctx.BlockHeight() + 50 | ||
priceVoteFromVal(0, block, TEST_ERATE) | ||
priceVoteFromVal(1, block, TEST_ERATE) | ||
input.Ctx = input.Ctx.WithBlockHeight(block + 100) | ||
|
||
input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
// input.OracleKeeper.SlashAndResetMissCounters(input.Ctx) | ||
// input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
require.Equal(t, i+1, input.OracleKeeper.MissCounters.GetOr(input.Ctx, ValAddrs[0], 0)) | ||
} | ||
allowedMissPct := sdk.OneDec().Sub(minValidPerWindow) | ||
allowedMissPeriods := allowedMissPct.MulInt64(votePeriodsPerWindow). | ||
TruncateInt64() | ||
|
||
validator := input.StakingKeeper.Validator(input.Ctx, ValAddrs[0]) | ||
require.Equal(t, stakingAmt, validator.GetBondedTokens()) | ||
// TODO | ||
|
||
// one more miss vote will inccur Account 1 slashing | ||
t.Logf("For %v blocks, valoper0 does not vote, while 1 and 2 do.", allowedMissPeriods) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you're conflating blocks with vote periods here. One vote period is many blocks (by default 30 on testnet), so if the goal is to vote once every vote period, we have to skip the block number by 30 in each iteration. However, this test doesn't really test validator slashing, only that the MissCounter doesn't increment. The slashing happens in the abci There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the name is wrong because it used to be a test about slashing that had abstain votes. Since that doesn't cause slashing, the test is no longer about that either. Will need to repurpose it |
||
for idxMissPeriod := uint64(0); idxMissPeriod < uint64(allowedMissPeriods); idxMissPeriod++ { | ||
block := input.Ctx.BlockHeight() + 1 | ||
input.Ctx = input.Ctx.WithBlockHeight(block) | ||
|
||
// Account 2, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 1) | ||
// Account 3, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 2) | ||
valIdx := 0 | ||
priceVoteFromVal(valIdx+1, block, TEST_ERATE) | ||
priceVoteFromVal(valIdx+2, block, TEST_ERATE) | ||
|
||
input.Ctx = input.Ctx.WithBlockHeight(votePeriodsPerWindow - 1) | ||
input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
input.OracleKeeper.SlashAndResetMissCounters(input.Ctx) | ||
// Valoper 0 votes outside band (miss) | ||
|
||
perfs := input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
missCount := input.OracleKeeper.MissCounters.GetOr(input.Ctx, ValAddrs[0], 0) | ||
require.EqualValues(t, 0, missCount, perfs.String()) | ||
} | ||
|
||
t.Log("valoper0 should not be slashed") | ||
validator := input.StakingKeeper.Validator(input.Ctx, ValAddrs[0]) | ||
require.Equal(t, TEST_STAKING_AMT, validator.GetBondedTokens()) | ||
|
||
// // one more miss vote will inccur Account 1 slashing | ||
Unique-Divine marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// | ||
// // Account 2, govstable | ||
// MakeAggregatePrevoteAndVote(t, input, msgServer, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 1) | ||
// // Account 3, govstable | ||
// MakeAggregatePrevoteAndVote(t, input, msgServer, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 2) | ||
// | ||
// input.Ctx = input.Ctx.WithBlockHeight(votePeriodsPerWindow - 1) | ||
// input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
validator = input.StakingKeeper.Validator(input.Ctx, ValAddrs[0]) | ||
require.Equal(t, sdk.OneDec().Sub(slashFraction).MulInt(stakingAmt).TruncateInt(), validator.GetBondedTokens()) | ||
// input.OracleKeeper.SlashAndResetMissCounters(input.Ctx) | ||
// // input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
// validator = input.StakingKeeper.Validator(input.Ctx, ValAddrs[0]) | ||
// require.Equal(t, sdk.OneDec().Sub(slashFraction).MulInt(stakingAmt).TruncateInt(), validator.GetBondedTokens()) | ||
} | ||
|
||
func TestNotPassedBallotSlashing(t *testing.T) { | ||
|
@@ -218,7 +244,7 @@ func TestNotPassedBallotSlashing(t *testing.T) { | |
input.Ctx = input.Ctx.WithBlockHeight(input.Ctx.BlockHeight() + 1) | ||
|
||
// Account 1, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 0) | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}}, 0) | ||
|
||
input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
input.OracleKeeper.SlashAndResetMissCounters(input.Ctx) | ||
|
@@ -248,13 +274,13 @@ func TestAbstainSlashing(t *testing.T) { | |
input.Ctx = input.Ctx.WithBlockHeight(input.Ctx.BlockHeight() + 1) | ||
|
||
// Account 1, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 0) | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}}, 0) | ||
|
||
// Account 2, govstable, abstain vote | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: sdk.ZeroDec()}}, 1) | ||
|
||
// Account 3, govstable | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: randomExchangeRate}}, 2) | ||
MakeAggregatePrevoteAndVote(t, input, h, 0, types.ExchangeRateTuples{{Pair: asset.Registry.Pair(denoms.NIBI, denoms.NUSD), ExchangeRate: TEST_ERATE}}, 2) | ||
|
||
input.OracleKeeper.UpdateExchangeRates(input.Ctx) | ||
input.OracleKeeper.SlashAndResetMissCounters(input.Ctx) | ||
|
@@ -263,5 +289,5 @@ func TestAbstainSlashing(t *testing.T) { | |
} | ||
|
||
validator := input.StakingKeeper.Validator(input.Ctx, ValAddrs[1]) | ||
require.Equal(t, stakingAmt, validator.GetBondedTokens()) | ||
require.Equal(t, TEST_STAKING_AMT, validator.GetBondedTokens()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bad idea, to have a return only used for tests. The function already mentions that it mutates the validator perfomances, returning it is confusing.