fix(oracle): power vote calculation (#1852)

* fix(oracle): power vote calculation

* finish test

* oracle: add SetVoteThreshold method

* more VoteThreshold tests

* lint

* cleanup

* Update x/oracle/types/params.go

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>

* comment changes only (review)

* combine loops

* fix simulations

* fix error test

* fix sims?

* review

---------

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>
(cherry picked from commit 76fc6f1)

x/oracle/abci.go

@@ -38,16 +38,12 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error {
 	validatorClaimMap := make(map[string]types.Claim)
 	powerReduction := k.StakingKeeper.PowerReduction(ctx)
 	// Calculate total validator power
-	var totalBondedValidatorPower int64
-	for _, v := range k.StakingKeeper.GetBondedValidatorsByPower(ctx) {
-		totalBondedValidatorPower += v.GetConsensusPower(powerReduction)
-	}
+	var totalBondedPower int64
 	for _, v := range k.StakingKeeper.GetBondedValidatorsByPower(ctx) {
 		addr := v.GetOperator()
-		validatorPowerRatio := sdk.NewDec(v.GetConsensusPower(powerReduction)).QuoInt64(totalBondedValidatorPower)
-		// Power is tracked as an int64 ranging from 0-100
-		validatorPower := validatorPowerRatio.MulInt64(100).RoundInt64()
-		validatorClaimMap[addr.String()] = types.NewClaim(validatorPower, 0, 0, addr)
+		power := v.GetConsensusPower(powerReduction)
+		totalBondedPower += power
+		validatorClaimMap[addr.String()] = types.NewClaim(power, 0, 0, addr)
 	}
 
 	// voteTargets defines the symbol (ticker) denoms that we require votes on
@@ -62,11 +58,14 @@ func CalcPrices(ctx sdk.Context, params types.Params, k keeper.Keeper) error {
 
 	// NOTE: it filters out inactive or jailed validators
 	ballotDenomSlice := k.OrganizeBallotByDenom(ctx, validatorClaimMap)
+	threshold := k.VoteThreshold(ctx).MulInt64(types.MaxVoteThresholdMultiplier).TruncateInt64()
 
 	// Iterate through ballots and update exchange rates; drop if not enough votes have been achieved.
 	for _, ballotDenom := range ballotDenomSlice {
-		// Convert ballot power to a percentage to compare with VoteThreshold param
-		if sdk.NewDecWithPrec(ballotDenom.Ballot.Power(), 2).LTE(k.VoteThreshold(ctx)) {
+		// Calculate the portion of votes received as an integer, scaled up using the
+		// same multiplier as the `threshold` computed above
+		support := ballotDenom.Ballot.Power() * types.MaxVoteThresholdMultiplier / totalBondedPower
+		if support < threshold {
 			ctx.Logger().Info("Ballot voting power is under vote threshold, dropping ballot", "denom", ballotDenom)
 			continue
 		}

x/oracle/abci_test.go

@@ -35,40 +35,50 @@ type IntegrationTestSuite struct {
 }
 
 const (
-	initialPower = int64(10000)
+	initialPower = int64(1000)
 )
 
 func (s *IntegrationTestSuite) SetupTest() {
 	require := s.Require()
 	isCheckTx := false
 	app := umeeapp.Setup(s.T())
-	ctx := app.BaseApp.NewContext(isCheckTx, tmproto.Header{
+	ctx := app.NewContext(isCheckTx, tmproto.Header{
 		ChainID: fmt.Sprintf("test-chain-%s", tmrand.Str(4)),
 	})
 
 	oracle.InitGenesis(ctx, app.OracleKeeper, *types.DefaultGenesisState())
 
+	// validate setup... umeeapp.Setup creates one validator, with 1uumee self delegation
+	setupVals := app.StakingKeeper.GetBondedValidatorsByPower(ctx)
+	s.Require().Len(setupVals, 1)
+	s.Require().Equal(int64(1), setupVals[0].GetConsensusPower(app.StakingKeeper.PowerReduction(ctx)))
+
 	sh := teststaking.NewHelper(s.T(), ctx, *app.StakingKeeper)
 	sh.Denom = bondDenom
 
-	// mint and send coins to validator
-	require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins))
+	// mint and send coins to validators
+	require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins.MulInt(sdk.NewIntFromUint64(3))))
 	require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr1, initCoins))
-	require.NoError(app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, initCoins))
 	require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr2, initCoins))
+	require.NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr3, initCoins))
 
-	sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 7000, true)
-	sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 3000, true)
+	// self delegate 999 in total ... 1 val with 1uumee is already created in umeeapp.Setup
+	sh.CreateValidatorWithValPower(valAddr1, valPubKey1, 599, true)
+	sh.CreateValidatorWithValPower(valAddr2, valPubKey2, 398, true)
+	sh.CreateValidatorWithValPower(valAddr3, valPubKey3, 2, true)
 
 	staking.EndBlocker(ctx, *app.StakingKeeper)
 
+	err := app.OracleKeeper.SetVoteThreshold(ctx, sdk.MustNewDecFromStr("0.4"))
+	s.Require().NoError(err)
+
 	s.app = app
 	s.ctx = ctx
 }
 
 // Test addresses
 var (
-	valPubKeys = simapp.CreateTestPubKeys(2)
+	valPubKeys = simapp.CreateTestPubKeys(3)
 
 	valPubKey1 = valPubKeys[0]
 	pubKey1    = secp256k1.GenPrivKey().PubKey()
@@ -80,24 +90,25 @@ var (
 	addr2      = sdk.AccAddress(pubKey2.Address())
 	valAddr2   = sdk.ValAddress(pubKey2.Address())
 
+	valPubKey3 = valPubKeys[2]
+	pubKey3    = secp256k1.GenPrivKey().PubKey()
+	addr3      = sdk.AccAddress(pubKey3.Address())
+	valAddr3   = sdk.ValAddress(pubKey3.Address())
+
 	initTokens = sdk.TokensFromConsensusPower(initialPower, sdk.DefaultPowerReduction)
 	initCoins  = sdk.NewCoins(sdk.NewCoin(bondDenom, initTokens))
 )
 
 func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() {
 	app, ctx := s.app, s.ctx
-	originalBlockHeight := ctx.BlockHeight()
 	ctx = ctx.WithBlockHeight(1)
 	preVoteBlockDiff := int64(app.OracleKeeper.VotePeriod(ctx) / 2)
 	voteBlockDiff := int64(app.OracleKeeper.VotePeriod(ctx)/2 + 1)
 
 	var (
-		val1Tuples   types.ExchangeRateTuples
-		val2Tuples   types.ExchangeRateTuples
-		val1PreVotes types.AggregateExchangeRatePrevote
-		val2PreVotes types.AggregateExchangeRatePrevote
-		val1Votes    types.AggregateExchangeRateVote
-		val2Votes    types.AggregateExchangeRateVote
+		val1Tuples types.ExchangeRateTuples
+		val2Tuples types.ExchangeRateTuples
+		val3Tuples types.ExchangeRateTuples
 	)
 	for _, denom := range app.OracleKeeper.AcceptList(ctx) {
 		val1Tuples = append(val1Tuples, types.ExchangeRateTuple{
@@ -108,36 +119,39 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() {
 			Denom:        denom.SymbolDenom,
 			ExchangeRate: sdk.MustNewDecFromStr("0.5"),
 		})
+		val3Tuples = append(val3Tuples, types.ExchangeRateTuple{
+			Denom:        denom.SymbolDenom,
+			ExchangeRate: sdk.MustNewDecFromStr("0.6"),
+		})
 	}
 
-	val1PreVotes = types.AggregateExchangeRatePrevote{
-		Hash:        "hash1",
-		Voter:       valAddr1.String(),
-		SubmitBlock: uint64(ctx.BlockHeight()),
-	}
-	val2PreVotes = types.AggregateExchangeRatePrevote{
-		Hash:        "hash2",
-		Voter:       valAddr2.String(),
-		SubmitBlock: uint64(ctx.BlockHeight()),
-	}
-
-	val1Votes = types.AggregateExchangeRateVote{
-		ExchangeRateTuples: val1Tuples,
-		Voter:              valAddr1.String(),
-	}
-	val2Votes = types.AggregateExchangeRateVote{
-		ExchangeRateTuples: val2Tuples,
-		Voter:              valAddr2.String(),
+	createVote := func(hash string, val sdk.ValAddress, rates types.ExchangeRateTuples, blockHeight uint64) (types.AggregateExchangeRatePrevote, types.AggregateExchangeRateVote) {
+		preVote := types.AggregateExchangeRatePrevote{
+			Hash:        "hash1",
+			Voter:       val.String(),
+			SubmitBlock: uint64(blockHeight),
+		}
+		vote := types.AggregateExchangeRateVote{
+			ExchangeRateTuples: rates,
+			Voter:              val.String(),
+		}
+		return preVote, vote
 	}
+	h := uint64(ctx.BlockHeight())
+	val1PreVotes, val1Votes := createVote("hash1", valAddr1, val1Tuples, h)
+	val2PreVotes, val2Votes := createVote("hash2", valAddr2, val2Tuples, h)
+	val3PreVotes, val3Votes := createVote("hash3", valAddr3, val3Tuples, h)
 
 	// total voting power per denom is 100%
 	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr1, val1PreVotes)
 	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes)
+	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr3, val3PreVotes)
 	oracle.EndBlocker(ctx, app.OracleKeeper)
 
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + voteBlockDiff)
 	app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr1, val1Votes)
 	app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr2, val2Votes)
+	app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr3, val3Votes)
 	oracle.EndBlocker(ctx, app.OracleKeeper)
 
 	for _, denom := range app.OracleKeeper.AcceptList(ctx) {
@@ -146,12 +160,13 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() {
 		s.Require().Equal(sdk.MustNewDecFromStr("1.0"), rate)
 	}
 
-	// update prevotes' block
+	// Test: only val2 votes (has 39% vote power).
+	// Total voting power per denom must be bigger or equal than 40% (see SetupTest).
+	// So if only val2 votes, we won't have any prices next block.
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff)
-	val1PreVotes.SubmitBlock = uint64(ctx.BlockHeight())
-	val2PreVotes.SubmitBlock = uint64(ctx.BlockHeight())
+	h = uint64(ctx.BlockHeight())
+	val2PreVotes.SubmitBlock = h
 
-	// total voting power per denom is 30%
 	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes)
 	oracle.EndBlocker(ctx, app.OracleKeeper)
 
@@ -165,30 +180,50 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() {
 		s.Require().Equal(sdk.ZeroDec(), rate)
 	}
 
-	// update prevotes' block
+	// Test: val2 and val3 votes.
+	// now we will have 40% of the power, so now we should have prices
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff)
+	h = uint64(ctx.BlockHeight())
+	val2PreVotes.SubmitBlock = h
+	val3PreVotes.SubmitBlock = h
+
+	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes)
+	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr3, val3PreVotes)
+	oracle.EndBlocker(ctx, app.OracleKeeper)
+
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + voteBlockDiff)
+	app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr2, val2Votes)
+	app.OracleKeeper.SetAggregateExchangeRateVote(ctx, valAddr3, val3Votes)
+	oracle.EndBlocker(ctx, app.OracleKeeper)
+
+	for _, denom := range app.OracleKeeper.AcceptList(ctx) {
+		rate, err := app.OracleKeeper.GetExchangeRate(ctx, denom.SymbolDenom)
+		s.Require().NoError(err)
+		s.Require().Equal(sdk.MustNewDecFromStr("0.5"), rate)
+	}
+
+	// TODO: check reward distribution
+	// https://github.com/umee-network/umee/issues/1853
+
+	// Test: val1 and val2 vote again
+	// umee has 69.9% power, and atom has 30%, so we should have price for umee, but not for atom
 	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + preVoteBlockDiff)
-	val1PreVotes.SubmitBlock = uint64(ctx.BlockHeight())
-	val2PreVotes.SubmitBlock = uint64(ctx.BlockHeight())
+	h = uint64(ctx.BlockHeight())
+	val1PreVotes.SubmitBlock = h
+	val2PreVotes.SubmitBlock = h
 
-	// umee has 100% power, and atom has 30%
-	val1Tuples = types.ExchangeRateTuples{
+	val1Votes.ExchangeRateTuples = types.ExchangeRateTuples{
 		types.ExchangeRateTuple{
 			Denom:        "umee",
 			ExchangeRate: sdk.MustNewDecFromStr("1.0"),
 		},
 	}
-	val2Tuples = types.ExchangeRateTuples{
-		types.ExchangeRateTuple{
-			Denom:        "umee",
-			ExchangeRate: sdk.MustNewDecFromStr("0.5"),
-		},
+	val2Votes.ExchangeRateTuples = types.ExchangeRateTuples{
 		types.ExchangeRateTuple{
 			Denom:        "atom",
 			ExchangeRate: sdk.MustNewDecFromStr("0.5"),
 		},
 	}
-	val1Votes.ExchangeRateTuples = val1Tuples
-	val2Votes.ExchangeRateTuples = val2Tuples
 
 	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr1, val1PreVotes)
 	app.OracleKeeper.SetAggregateExchangeRatePrevote(ctx, valAddr2, val2PreVotes)
@@ -205,8 +240,6 @@ func (s *IntegrationTestSuite) TestEndBlockerVoteThreshold() {
 	rate, err = app.OracleKeeper.GetExchangeRate(ctx, "atom")
 	s.Require().ErrorIs(err, sdkerrors.Wrap(types.ErrUnknownDenom, "atom"))
 	s.Require().Equal(sdk.ZeroDec(), rate)
-
-	ctx = ctx.WithBlockHeight(originalBlockHeight)
 }
 
 var exchangeRates = map[string][]sdk.Dec{

x/oracle/keeper/ballot.go

@@ -21,13 +21,10 @@ func (k Keeper) OrganizeBallotByDenom(
 		// organize ballot only for the active validators
 		claim, ok := validatorClaimMap[vote.Voter]
 		if ok {
-			power := claim.Power
-
 			for _, tuple := range vote.ExchangeRateTuples {
-				tmpPower := power
 				votes[tuple.Denom] = append(
 					votes[tuple.Denom],
-					types.NewVoteForTally(tuple.ExchangeRate, tuple.Denom, voterAddr, tmpPower),
+					types.NewVoteForTally(tuple.ExchangeRate, tuple.Denom, voterAddr, claim.Power),
 				)
 			}
 		}

x/oracle/keeper/params.go

@@ -12,13 +12,24 @@ func (k Keeper) VotePeriod(ctx sdk.Context) (res uint64) {
 	return
 }
 
-// VoteThreshold returns the minimum percentage of votes that must be received
-// for a ballot to pass.
+// VoteThreshold returns the minimum portion of combined validator power of votes
+// that must be received for a ballot to pass.
 func (k Keeper) VoteThreshold(ctx sdk.Context) (res sdk.Dec) {
 	k.paramSpace.Get(ctx, types.KeyVoteThreshold, &res)
 	return
 }
 
+// SetVoteThreshold sets min combined validator power voting on a denom to accept
+// it as valid.
+// TODO: this is used in tests, we should refactor the way how this is handled.
+func (k Keeper) SetVoteThreshold(ctx sdk.Context, threshold sdk.Dec) error {
+	if err := types.ValidateVoteThreshold(threshold); err != nil {
+		return err
+	}
+	k.paramSpace.Set(ctx, types.KeyVoteThreshold, &threshold)
+	return nil
+}
+
 // RewardBand returns the ratio of allowable exchange rate error that a validator
 // can be rewarded.
 func (k Keeper) RewardBand(ctx sdk.Context) (res sdk.Dec) {

x/oracle/simulations/genesis.go

@@ -30,9 +30,9 @@ func GenVotePeriod(r *rand.Rand) uint64 {
 	return uint64(5 + r.Intn(100))
 }
 
-// GenVoteThreshold produces a randomized VoteThreshold in the range of [0.333, 0.666]
+// GenVoteThreshold produces a randomized VoteThreshold in the range of [0.34, 0.67]
 func GenVoteThreshold(r *rand.Rand) sdk.Dec {
-	return sdk.NewDecWithPrec(333, 3).Add(sdk.NewDecWithPrec(int64(r.Intn(333)), 3))
+	return sdk.NewDecWithPrec(34, 2).Add(sdk.NewDecWithPrec(int64(r.Intn(33)), 2))
 }
 
 // GenRewardBand produces a randomized RewardBand in the range of [0.000, 0.100]

x/oracle/types/params.go

@@ -4,10 +4,22 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
 	"gopkg.in/yaml.v3"
 )
 
+var (
+	oneDec           = sdk.OneDec()
+	minVoteThreshold = sdk.NewDecWithPrec(33, 2) // 0.33
+)
+
+// maxium number of decimals allowed for VoteThreshold
+const (
+	MaxVoteThresholdPrecision  = 2
+	MaxVoteThresholdMultiplier = 100 // must be 10^MaxVoteThresholdPrecision
+)
+
 // Parameter keys
 var (
 	KeyVotePeriod               = []byte("VotePeriod")
@@ -220,16 +232,7 @@ func validateVoteThreshold(i interface{}) error {
 	if !ok {
 		return fmt.Errorf("invalid parameter type: %T", i)
 	}
-
-	if v.LT(sdk.NewDecWithPrec(33, 2)) {
-		return fmt.Errorf("vote threshold must be bigger than 33%%: %s", v)
-	}
-
-	if v.GT(sdk.OneDec()) {
-		return fmt.Errorf("vote threshold too large: %s", v)
-	}
-
-	return nil
+	return ValidateVoteThreshold(v)
 }
 
 func validateRewardBand(i interface{}) error {
@@ -378,3 +381,19 @@ func validateMaximumMedianStamps(i interface{}) error {
 
 	return nil
 }
+
+// ValidateVoteThreshold validates oracle exchange rates power vote threshold.
+// Must be
+// * a decimal value > 0.33 and <= 1.
+// * max precision is 2 (so 0.501 is not allowed)
+func ValidateVoteThreshold(x sdk.Dec) error {
+	if x.LTE(minVoteThreshold) || x.GT(oneDec) {
+		return sdkerrors.ErrInvalidRequest.Wrapf("threshold must be bigger than %s and <= 1", minVoteThreshold)
+	}
+	i := x.MulInt64(100).TruncateInt64()
+	x2 := sdk.NewDecWithPrec(i, MaxVoteThresholdPrecision)
+	if !x2.Equal(x) {
+		return sdkerrors.ErrInvalidRequest.Wrap("threshold precision must be maximum 2 decimals")
+	}
+	return nil
+}