fix(oracle)!: count vote omission as abstain for less slashing + more stability

CHANGELOG.md

@@ -78,6 +78,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#1459](https://github.com/NibiruChain/nibiru/pull/1459) - fix(spot): wire `x/spot` msgService into app router
 * [#1467](https://github.com/NibiruChain/nibiru/pull/1467) - fix(oracle): make `calcTwap` safer
 * [#1464](https://github.com/NibiruChain/nibiru/pull/1464) - fix(gov): wire legacy proposal handlers
+* [#1565](https://github.com/NibiruChain/nibiru/pull/1565) - fix(oracle)!: Count vote omission as abstain for less slashing + more stability
 
 ### State Machine Breaking
 
@@ -640,4 +641,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Testing
 
 * [#695](https://github.com/NibiruChain/nibiru/pull/695) Add `OpenPosition` integration tests.
-* [#692](https://github.com/NibiruChain/nibiru/pull/692) Add test coverage for Perp MsgServer methods.
+* [#692](https://github.com/NibiruChain/nibiru/pull/692) Add test coverage for Perp MsgServer methods.

x/oracle/keeper/ballot.go

@@ -22,15 +22,15 @@ import (
 // asset pair is associated with its collection of ExchangeRateBallots.
 func (k Keeper) groupBallotsByPair(
 	ctx sdk.Context,
-	validatorsPerformance types.ValidatorPerformances,
+	valPerfMap types.ValidatorPerformances,
 ) (pairBallotsMap map[asset.Pair]types.ExchangeRateBallots) {
 	pairBallotsMap = map[asset.Pair]types.ExchangeRateBallots{}
 
 	for _, value := range k.Votes.Iterate(ctx, collections.Range[sdk.ValAddress]{}).KeyValues() {
 		voterAddr, aggregateVote := value.Key, value.Value
 
 		// organize ballot only for the active validators
-		if validatorPerformance, exists := validatorsPerformance[aggregateVote.Voter]; exists {
+		if validatorPerformance, exists := valPerfMap[aggregateVote.Voter]; exists {
 			for _, exchangeRateTuple := range aggregateVote.ExchangeRateTuples {
 				power := validatorPerformance.Power
 				if !exchangeRateTuple.ExchangeRate.IsPositive() {
@@ -137,11 +137,17 @@ func (k Keeper) removeInvalidBallots(
 	return pairBallotsMap, whitelistedPairs
 }
 
-// Tally calculates the median and returns it. Sets the set of voters to be rewarded, i.e. voted within
-// a reasonable spread from the weighted median to the store
+// Tally calculates the median and returns it. Sets the set of voters to be
+// rewarded, i.e. voted within a reasonable spread from the weighted median to
+// the store.
 //
-// ALERT: This function mutates validatorPerformances slice based on the votes made by the validators.
-func Tally(ballots types.ExchangeRateBallots, rewardBand sdk.Dec, validatorPerformances types.ValidatorPerformances) sdk.Dec {
+// ALERT: This function mutates validatorPerformances slice based on the votes
+// made by the validators.
+func Tally(
+	ballots types.ExchangeRateBallots,
+	rewardBand sdk.Dec,
+	validatorPerformances types.ValidatorPerformances,
+) (sdk.Dec, types.ValidatorPerformances) {
 	weightedMedian := ballots.WeightedMedianWithAssertion()
 	standardDeviation := ballots.StandardDeviation(weightedMedian)
 	rewardSpread := weightedMedian.Mul(rewardBand.QuoInt64(2))
@@ -155,16 +161,19 @@ func Tally(ballots types.ExchangeRateBallots, rewardBand sdk.Dec, validatorPerfo
 		voteInsideSpread := ballot.ExchangeRate.GTE(weightedMedian.Sub(rewardSpread)) &&
 			ballot.ExchangeRate.LTE(weightedMedian.Add(rewardSpread))
 		isAbstainVote := !ballot.ExchangeRate.IsPositive()
-
-		if voteInsideSpread || isAbstainVote {
-			voterAddr := ballot.Voter.String()
-
-			validatorPerformance := validatorPerformances[voterAddr]
+		isMiss := !(voteInsideSpread || isAbstainVote)
+		voterAddr := ballot.Voter.String()
+		validatorPerformance := validatorPerformances[voterAddr]
+		switch {
+		case voteInsideSpread:
 			validatorPerformance.RewardWeight += ballot.Power
 			validatorPerformance.WinCount++
-			validatorPerformances[voterAddr] = validatorPerformance
+		case isMiss:
+			validatorPerformance.MissCount++
+		case isAbstainVote:
+			validatorPerformance.AbstainCount++
 		}
+		validatorPerformances[voterAddr] = validatorPerformance
 	}
-
-	return weightedMedian
+	return weightedMedian, validatorPerformances
 }

x/oracle/keeper/ballot_test.go

@@ -37,15 +37,17 @@ func TestGroupBallotsByPair(t *testing.T) {
 	require.NoError(t, err)
 	staking.EndBlocker(fixture.Ctx, &fixture.StakingKeeper)
 
+	pairBtc := asset.Registry.Pair(denoms.BTC, denoms.NUSD)
+	pairEth := asset.Registry.Pair(denoms.ETH, denoms.NUSD)
 	btcBallots := types.ExchangeRateBallots{
-		types.NewExchangeRateBallot(sdk.NewDec(17), asset.Registry.Pair(denoms.BTC, denoms.NUSD), ValAddrs[0], power),
-		types.NewExchangeRateBallot(sdk.NewDec(10), asset.Registry.Pair(denoms.BTC, denoms.NUSD), ValAddrs[1], power),
-		types.NewExchangeRateBallot(sdk.NewDec(6), asset.Registry.Pair(denoms.BTC, denoms.NUSD), ValAddrs[2], power),
+		{Pair: pairBtc, ExchangeRate: sdk.NewDec(17), Voter: ValAddrs[0], Power: power},
+		{Pair: pairBtc, ExchangeRate: sdk.NewDec(10), Voter: ValAddrs[1], Power: power},
+		{Pair: pairBtc, ExchangeRate: sdk.NewDec(6), Voter: ValAddrs[2], Power: power},
 	}
 	ethBallots := types.ExchangeRateBallots{
-		types.NewExchangeRateBallot(sdk.NewDec(1000), asset.Registry.Pair(denoms.ETH, denoms.NUSD), ValAddrs[0], power),
-		types.NewExchangeRateBallot(sdk.NewDec(1300), asset.Registry.Pair(denoms.ETH, denoms.NUSD), ValAddrs[1], power),
-		types.NewExchangeRateBallot(sdk.NewDec(2000), asset.Registry.Pair(denoms.ETH, denoms.NUSD), ValAddrs[2], power),
+		{Pair: pairEth, ExchangeRate: sdk.NewDec(1_000), Voter: ValAddrs[0], Power: power},
+		{Pair: pairEth, ExchangeRate: sdk.NewDec(1_300), Voter: ValAddrs[1], Power: power},
+		{Pair: pairEth, ExchangeRate: sdk.NewDec(2_000), Voter: ValAddrs[2], Power: power},
 	}
 
 	for i, ballot := range btcBallots {

x/oracle/keeper/msg_server_test.go

@@ -17,7 +17,7 @@ func TestFeederDelegation(t *testing.T) {
 	exchangeRates := types.ExchangeRateTuples{
 		{
 			Pair:         asset.Registry.Pair(denoms.BTC, denoms.NUSD),
-			ExchangeRate: randomExchangeRate,
+			ExchangeRate: testExchangeRate,
 		},
 	}
 

x/oracle/keeper/reward_test.go

@@ -40,7 +40,7 @@ func TestKeeperRewardsDistributionMultiVotePeriods(t *testing.T) {
 			MakeAggregatePrevoteAndVote(t, fixture, msgServer, fixture.Ctx.BlockHeight(), types.ExchangeRateTuples{
 				{
 					Pair:         asset.Registry.Pair(denoms.NIBI, denoms.NUSD),
-					ExchangeRate: randomExchangeRate,
+					ExchangeRate: testExchangeRate,
 				},
 			}, valIndex)
 		}

x/oracle/keeper/slash_test.go

@@ -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: testExchangeRate},
 		}, 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: testExchangeRate.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: testExchangeRate},
 		}, 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: testExchangeRate},
 		}, 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, testStakingAmt, 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: testExchangeRate},
 	}, 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: testExchangeRate.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: testExchangeRate},
 	}, 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: testExchangeRate},
 	}, 3)
 
 	input.Ctx = input.Ctx.WithBlockHeight(votePeriodsPerWindow - 1)
@@ -160,46 +160,47 @@ 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(testStakingAmt).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)
-	minValidPerWindow := input.OracleKeeper.MinValidPerWindow(input.Ctx)
+	votePeriodsPerSlashWindow := sdk.NewDec(int64(input.OracleKeeper.SlashWindow(input.Ctx))).QuoInt64(int64(input.OracleKeeper.VotePeriod(input.Ctx))).TruncateInt64()
+	minValidVotePeriodsPerWindow := 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)
-
-		// 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)
-
-		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))
+	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())
+
+	allowedMissPct := sdk.OneDec().Sub(minValidVotePeriodsPerWindow)
+	allowedMissVotePeriods := allowedMissPct.MulInt64(votePeriodsPerSlashWindow).
+		TruncateInt64()
+	t.Logf("For %v blocks, valoper0 does not vote, while 1 and 2 do.", allowedMissVotePeriods)
+	for idxMissPeriod := uint64(0); idxMissPeriod < uint64(allowedMissVotePeriods); idxMissPeriod++ {
+		block := input.Ctx.BlockHeight() + 1
+		input.Ctx = input.Ctx.WithBlockHeight(block)
+
+		valIdx := 0 // Valoper doesn't vote (abstain)
+		priceVoteFromVal(valIdx+1, block, testExchangeRate)
+		priceVoteFromVal(valIdx+2, block, testExchangeRate)
+
+		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, stakingAmt, validator.GetBondedTokens())
-
-	// one more miss vote will inccur Account 1 slashing
-
-	// 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)
-
-	input.Ctx = input.Ctx.WithBlockHeight(votePeriodsPerWindow - 1)
-	input.OracleKeeper.UpdateExchangeRates(input.Ctx)
-	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())
+	require.Equal(t, testStakingAmt, validator.GetBondedTokens())
 }
 
 func TestNotPassedBallotSlashing(t *testing.T) {
@@ -218,7 +219,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: testExchangeRate}}, 0)
 
 	input.OracleKeeper.UpdateExchangeRates(input.Ctx)
 	input.OracleKeeper.SlashAndResetMissCounters(input.Ctx)
@@ -248,13 +249,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: testExchangeRate}}, 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: testExchangeRate}}, 2)
 
 		input.OracleKeeper.UpdateExchangeRates(input.Ctx)
 		input.OracleKeeper.SlashAndResetMissCounters(input.Ctx)
@@ -263,5 +264,5 @@ func TestAbstainSlashing(t *testing.T) {
 	}
 
 	validator := input.StakingKeeper.Validator(input.Ctx, ValAddrs[1])
-	require.Equal(t, stakingAmt, validator.GetBondedTokens())
+	require.Equal(t, testStakingAmt, validator.GetBondedTokens())
 }

x/oracle/keeper/test_utils.go

@@ -285,8 +285,8 @@ func AllocateRewards(t *testing.T, input TestFixture, rewards sdk.Coins, votePer
 }
 
 var (
-	stakingAmt         = sdk.TokensFromConsensusPower(10, sdk.DefaultPowerReduction)
-	randomExchangeRate = sdk.NewDec(1700)
+	testStakingAmt   = sdk.TokensFromConsensusPower(10, sdk.DefaultPowerReduction)
+	testExchangeRate = sdk.NewDec(1700)
 )
 
 func Setup(t *testing.T) (TestFixture, types.MsgServer) {
@@ -304,22 +304,29 @@ func Setup(t *testing.T) (TestFixture, types.MsgServer) {
 	sh := stakingkeeper.NewMsgServerImpl(&fixture.StakingKeeper)
 
 	// Validator created
-	_, err := sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[0], ValPubKeys[0], stakingAmt))
+	_, err := sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[0], ValPubKeys[0], testStakingAmt))
 	require.NoError(t, err)
-	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[1], ValPubKeys[1], stakingAmt))
+	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[1], ValPubKeys[1], testStakingAmt))
 	require.NoError(t, err)
-	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[2], ValPubKeys[2], stakingAmt))
+	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[2], ValPubKeys[2], testStakingAmt))
 	require.NoError(t, err)
-	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[3], ValPubKeys[3], stakingAmt))
+	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[3], ValPubKeys[3], testStakingAmt))
 	require.NoError(t, err)
-	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[4], ValPubKeys[4], stakingAmt))
+	_, err = sh.CreateValidator(fixture.Ctx, NewTestMsgCreateValidator(ValAddrs[4], ValPubKeys[4], testStakingAmt))
 	require.NoError(t, err)
 	staking.EndBlocker(fixture.Ctx, &fixture.StakingKeeper)
 
 	return fixture, h
 }
 
-func MakeAggregatePrevoteAndVote(t *testing.T, input TestFixture, msgServer types.MsgServer, height int64, rates types.ExchangeRateTuples, valIdx int) {
+func MakeAggregatePrevoteAndVote(
+	t *testing.T,
+	input TestFixture,
+	msgServer types.MsgServer,
+	height int64,
+	rates types.ExchangeRateTuples,
+	valIdx int,
+) {
 	salt := "1"
 	ratesStr, err := rates.ToString()
 	require.NoError(t, err)