chore: oracle cosmetic updates (#1538)

* various oracle improvements

* various oracle improvements

* error message update

* Apply suggestions from code review

Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>

* rename

Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>

x/leverage/types/errors.go

@@ -20,7 +20,8 @@ var (
 	ErrSupplyNotAllowed     = sdkerrors.Register(ModuleName, 203, "supplying of Token disabled")
 	ErrBorrowNotAllowed     = sdkerrors.Register(ModuleName, 204, "borrowing of Token disabled")
 	ErrBlacklisted          = sdkerrors.Register(ModuleName, 205, "blacklisted Token")
-	ErrCollateralWeightZero = sdkerrors.Register(ModuleName, 206, "collateral weight of Token is zero")
+	ErrCollateralWeightZero = sdkerrors.Register(ModuleName, 206,
+		"collateral weight of Token is zero: can't be used as a collateral")
 
 	// 3XX = User Positions
 	ErrInsufficientBalance    = sdkerrors.Register(ModuleName, 300, "insufficient balance")

x/oracle/abci.go

@@ -63,12 +63,13 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) error {
 		claimSlice := types.ClaimMapToSlice(validatorClaimMap)
 		for _, claim := range claimSlice {
 			// Skip valid voters
-			if int(claim.WinCount) == voteTargetsLen {
+			// in MsgAggregateExchangeRateVote we filter tokens from the AcceptList.
+			if int(claim.TokensVoted) == voteTargetsLen {
 				continue
 			}
 
 			// Increase miss counter
-			k.SetMissCounter(ctx, claim.Recipient, k.GetMissCounter(ctx, claim.Recipient)+1)
+			k.SetMissCounter(ctx, claim.Validator, k.GetMissCounter(ctx, claim.Validator)+1)
 		}
 
 		// Distribute rewards to ballot winners
@@ -81,7 +82,7 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) error {
 		)
 
 		// Clear the ballot
-		k.ClearBallots(ctx, params.VotePeriod)
+		k.ClearVotes(ctx, params.VotePeriod)
 	}
 
 	// Slash oracle providers who missed voting over the threshold and
@@ -126,7 +127,7 @@ func Tally(
 			claim := validatorClaimMap[key]
 
 			claim.Weight += tallyVote.Power
-			claim.WinCount++
+			claim.TokensVoted++
 			validatorClaimMap[key] = claim
 		}
 	}

x/oracle/client/cli/tx.go

@@ -85,7 +85,7 @@ func GetCmdAggregateExchangeRatePrevote() *cobra.Command {
 				return err
 			}
 
-			hash, err := types.AggregateVoteHashFromHexString(args[0])
+			hash, err := types.AggregateVoteHashFromHex(args[0])
 			if err != nil {
 				return err
 			}

x/oracle/keeper/ballot.go

@@ -25,14 +25,12 @@ func (k Keeper) OrganizeBallotByDenom(
 
 			for _, tuple := range vote.ExchangeRateTuples {
 				tmpPower := power
-
 				votes[tuple.Denom] = append(
 					votes[tuple.Denom],
 					types.NewVoteForTally(tuple.ExchangeRate, tuple.Denom, voterAddr, tmpPower),
 				)
 			}
 		}
-
 		return false
 	}
 
@@ -46,13 +44,13 @@ func (k Keeper) OrganizeBallotByDenom(
 	return types.BallotMapToSlice(votes)
 }
 
-// ClearBallots clears all tallied prevotes and votes from the store.
-func (k Keeper) ClearBallots(ctx sdk.Context, votePeriod uint64) {
-	// clear all aggregate prevotes
+// ClearVotes clears all tallied prevotes and votes from the store.
+func (k Keeper) ClearVotes(ctx sdk.Context, votePeriod uint64) {
+	currentHeight := uint64(ctx.BlockHeight())
 	k.IterateAggregateExchangeRatePrevotes(
 		ctx,
 		func(voterAddr sdk.ValAddress, aggPrevote types.AggregateExchangeRatePrevote) bool {
-			if ctx.BlockHeight() > int64(aggPrevote.SubmitBlock+votePeriod) {
+			if currentHeight > (aggPrevote.SubmitBlock + votePeriod) {
 				k.DeleteAggregateExchangeRatePrevote(ctx, voterAddr)
 			}
 

x/oracle/keeper/ballot_test.go

@@ -28,10 +28,10 @@ func (s *IntegrationTestSuite) TestBallot_OrganizeBallotByDenom() {
 	)
 
 	claimMap[valAddr.String()] = types.Claim{
-		Power:     1,
-		Weight:    1,
-		WinCount:  1,
-		Recipient: valAddr,
+		Power:       1,
+		Weight:      1,
+		TokensVoted: 1,
+		Validator:   valAddr,
 	}
 	res = s.app.OracleKeeper.OrganizeBallotByDenom(s.ctx, claimMap)
 	require.Equal([]types.BallotDenom{
@@ -66,7 +66,7 @@ func (s *IntegrationTestSuite) TestBallot_ClearBallots() {
 	s.Require().NoError(err)
 	s.Require().Equal(voteRes, vote)
 
-	s.app.OracleKeeper.ClearBallots(s.ctx, 0)
+	s.app.OracleKeeper.ClearVotes(s.ctx, 0)
 	_, err = s.app.OracleKeeper.GetAggregateExchangeRatePrevote(s.ctx, valAddr)
 	s.Require().Error(err)
 	_, err = s.app.OracleKeeper.GetAggregateExchangeRateVote(s.ctx, valAddr)

x/oracle/keeper/msg_server.go

@@ -44,7 +44,7 @@ func (ms msgServer) AggregateExchangeRatePrevote(
 	}
 
 	// Convert hex string to votehash
-	voteHash, err := types.AggregateVoteHashFromHexString(msg.Hash)
+	voteHash, err := types.AggregateVoteHashFromHex(msg.Hash)
 	if err != nil {
 		return nil, sdkerrors.Wrap(types.ErrInvalidHash, err.Error())
 	}
@@ -78,23 +78,25 @@ func (ms msgServer) AggregateExchangeRateVote(
 		return nil, sdkerrors.Wrap(types.ErrNoAggregatePrevote, msg.Validator)
 	}
 
-	// Check a msg is submitted proper period
-	if (uint64(ctx.BlockHeight())/params.VotePeriod)-(aggregatePrevote.SubmitBlock/params.VotePeriod) != 1 {
+	// Check the vote is submitted in the `period == prevote.period+1`
+	if (uint64(ctx.BlockHeight()) / params.VotePeriod) != (aggregatePrevote.SubmitBlock/params.VotePeriod)+1 {
 		return nil, types.ErrRevealPeriodMissMatch
 	}
 
 	exchangeRateTuples, err := types.ParseExchangeRateTuples(msg.ExchangeRates)
 	if err != nil {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidCoins, err.Error())
+		return nil, err
 	}
 
-	// Verify a exchange rate with aggregate prevote hash
+	// Verify that the vote hash and prevote hash match
 	hash := types.GetAggregateVoteHash(msg.Salt, msg.ExchangeRates, valAddr)
 	if aggregatePrevote.Hash != hash.String() {
 		return nil, sdkerrors.Wrapf(types.ErrVerificationFailed, "must be given %s not %s", aggregatePrevote.Hash, hash)
 	}
 
 	// Filter out rates which aren't included in the AcceptList
+	// This is also needed for slashing; in the end blocker we are checking if validator voted
+	// for all required currencies. If they missed some, then we increase their slashing counter.
 	filteredTuples := types.ExchangeRateTuples{}
 	for _, tuple := range exchangeRateTuples {
 		if params.AcceptList.Contains(tuple.Denom) {

x/oracle/keeper/reward.go

@@ -36,7 +36,7 @@ func (k Keeper) RewardBallotWinners(
 		ballotPowerSum += winner.Weight
 	}
 
-	// return if the ballot is empty
+	// early return - ballot was empty
 	if ballotPowerSum == 0 {
 		return
 	}
@@ -62,7 +62,7 @@ func (k Keeper) RewardBallotWinners(
 	var distributedReward sdk.Coins
 
 	for _, winner := range ballotWinners {
-		receiverVal := k.StakingKeeper.Validator(ctx, winner.Recipient)
+		receiverVal := k.StakingKeeper.Validator(ctx, winner.Validator)
 		// in case absence of the validator, we just skip distribution
 		if receiverVal == nil {
 			continue

x/oracle/types/ballot.go

@@ -1,6 +1,7 @@
 package types
 
 import (
+	"bytes"
 	"sort"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -55,14 +56,12 @@ func (pb ExchangeRateBallot) WeightedMedian() (sdk.Dec, error) {
 	if !sort.IsSorted(pb) {
 		return sdk.ZeroDec(), ErrBallotNotSorted
 	}
-	totalPower := pb.Power()
 
 	if pb.Len() > 0 {
+		totalPower := pb.Power()
 		var pivot int64
 		for _, v := range pb {
-			votePower := v.Power
-
-			pivot += votePower
+			pivot += v.Power
 			if pivot >= (totalPower / 2) {
 				return v.ExchangeRate, nil
 			}
@@ -148,19 +147,19 @@ func BallotMapToSlice(votes map[string]ExchangeRateBallot) []BallotDenom {
 
 // Claim is an interface that directs its rewards to an attached bank account.
 type Claim struct {
-	Power     int64
-	Weight    int64
-	WinCount  int64
-	Recipient sdk.ValAddress
+	Power       int64
+	Weight      int64
+	TokensVoted int64
+	Validator   sdk.ValAddress
 }
 
 // NewClaim generates a Claim instance.
-func NewClaim(power, weight, winCount int64, recipient sdk.ValAddress) Claim {
+func NewClaim(power, weight, winCount int64, v sdk.ValAddress) Claim {
 	return Claim{
-		Power:     power,
-		Weight:    weight,
-		WinCount:  winCount,
-		Recipient: recipient,
+		Power:       power,
+		Weight:      weight,
+		TokensVoted: winCount,
+		Validator:   v,
 	}
 }
 
@@ -169,16 +168,11 @@ func ClaimMapToSlice(claims map[string]Claim) []Claim {
 	c := make([]Claim, len(claims))
 	i := 0
 	for _, claim := range claims {
-		c[i] = Claim{
-			Power:     claim.Power,
-			Weight:    claim.Weight,
-			WinCount:  claim.WinCount,
-			Recipient: claim.Recipient,
-		}
+		c[i] = claim // makes copy
 		i++
 	}
 	sort.Slice(c, func(i, j int) bool {
-		return c[i].Recipient.String() < c[j].Recipient.String()
+		return bytes.Compare(c[i].Validator, c[j].Validator) < 0
 	})
 	return c
 }

x/oracle/types/hash.go

@@ -5,6 +5,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/tendermint/tendermint/crypto/tmhash"
@@ -21,25 +22,13 @@ type AggregateVoteHash []byte
 // GetAggregateVoteHash computes hash value of ExchangeRateVote to avoid
 // redundant DecCoins stringify operation.
 func GetAggregateVoteHash(salt, exchangeRatesStr string, voter sdk.ValAddress) AggregateVoteHash {
-	hash := tmhash.NewTruncated()
-	sourceStr := fmt.Sprintf("%s:%s:%s", salt, exchangeRatesStr, voter.String())
-
-	if _, err := hash.Write([]byte(sourceStr)); err != nil {
-		panic(err)
-	}
-
-	bz := hash.Sum(nil)
-	return bz
+	sourceStr := strings.Join([]string{salt, exchangeRatesStr, voter.String()}, ":")
+	return tmhash.SumTruncated([]byte(sourceStr))
 }
 
-// AggregateVoteHashFromHexString convert hex string to AggregateVoteHash.
-func AggregateVoteHashFromHexString(s string) (AggregateVoteHash, error) {
-	h, err := hex.DecodeString(s)
-	if err != nil {
-		return nil, err
-	}
-
-	return h, nil
+// AggregateVoteHashFromHex convert hex string to AggregateVoteHash.
+func AggregateVoteHashFromHex(s string) (AggregateVoteHash, error) {
+	return hex.DecodeString(s)
 }
 
 // String implements fmt.Stringer interface
@@ -112,7 +101,7 @@ func (h *AggregateVoteHash) UnmarshalJSON(data []byte) error {
 		return err
 	}
 
-	h2, err := AggregateVoteHashFromHexString(s)
+	h2, err := AggregateVoteHashFromHex(s)
 	if err != nil {
 		return err
 	}

x/oracle/types/hash_test.go

@@ -17,7 +17,7 @@ func TestAggregateVoteHash(t *testing.T) {
 
 	aggregateVoteHash := GetAggregateVoteHash("salt", "UMEE:100,ATOM:100", sdk.ValAddress(addrs[0]))
 	hexStr := hex.EncodeToString(aggregateVoteHash)
-	aggregateVoteHashRes, err := AggregateVoteHashFromHexString(hexStr)
+	aggregateVoteHashRes, err := AggregateVoteHashFromHex(hexStr)
 	require.NoError(t, err)
 	require.Equal(t, aggregateVoteHash, aggregateVoteHashRes)
 	require.True(t, aggregateVoteHash.Equal(aggregateVoteHash))

x/oracle/types/msgs.go

@@ -41,7 +41,7 @@ func (msg MsgAggregateExchangeRatePrevote) GetSigners() []sdk.AccAddress {
 
 // ValidateBasic Implements sdk.Msg
 func (msg MsgAggregateExchangeRatePrevote) ValidateBasic() error {
-	_, err := AggregateVoteHashFromHexString(msg.Hash)
+	_, err := AggregateVoteHashFromHex(msg.Hash)
 	if err != nil {
 		return sdkerrors.Wrapf(ErrInvalidHash, "invalid vote hash (%s)", err)
 	}
@@ -124,7 +124,7 @@ func (msg MsgAggregateExchangeRateVote) ValidateBasic() error {
 	if len(msg.Salt) != 64 {
 		return ErrInvalidSaltLength
 	}
-	_, err = AggregateVoteHashFromHexString(msg.Salt)
+	_, err = AggregateVoteHashFromHex(msg.Salt)
 	if err != nil {
 		return sdkerrors.Wrap(ErrInvalidSaltFormat, "salt must be a valid hex string")
 	}

x/oracle/types/vote.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"gopkg.in/yaml.v3"
 
 	"github.com/umee-network/umee/v3/x/leverage/types"
@@ -16,8 +17,8 @@ func NewAggregateExchangeRatePrevote(
 	submitBlock uint64,
 ) AggregateExchangeRatePrevote {
 	return AggregateExchangeRatePrevote{
-		Hash:        hash.String(),
-		Voter:       voter.String(),
+		Hash:        hash.String(),  // we could store bytes here!
+		Voter:       voter.String(), // we could store bytes here!
 		SubmitBlock: submitBlock,
 	}
 }
@@ -34,7 +35,7 @@ func NewAggregateExchangeRateVote(
 ) AggregateExchangeRateVote {
 	return AggregateExchangeRateVote{
 		ExchangeRateTuples: exchangeRateTuples,
-		Voter:              voter.String(),
+		Voter:              voter.String(), // we could use bytes here!
 	}
 }
 
@@ -76,7 +77,7 @@ func ParseExchangeRateTuples(tuplesStr string) (ExchangeRateTuples, error) {
 	tupleStrs := strings.Split(tuplesStr, ",")
 	tuples := make(ExchangeRateTuples, len(tupleStrs))
 
-	duplicateCheckMap := make(map[string]bool)
+	duplicateCheckMap := make(map[string]struct{})
 	for i, tupleStr := range tupleStrs {
 		denomAmountStr := strings.Split(tupleStr, ":")
 		if len(denomAmountStr) != 2 {
@@ -92,17 +93,15 @@ func ParseExchangeRateTuples(tuplesStr string) (ExchangeRateTuples, error) {
 		}
 
 		denom := strings.ToUpper(denomAmountStr[0])
-
 		tuples[i] = ExchangeRateTuple{
 			Denom:        denom,
 			ExchangeRate: decCoin,
 		}
 
 		if _, ok := duplicateCheckMap[denom]; ok {
-			return nil, fmt.Errorf("duplicated denom %s", denom)
+			return nil, sdkerrors.ErrInvalidCoins.Wrapf("duplicated denom %s", denom)
 		}
-
-		duplicateCheckMap[denom] = true
+		duplicateCheckMap[denom] = struct{}{}
 	}
 
 	return tuples, nil