[Feature] Apply audit (#504)

* move oracle SlashWindow check to outside of VotePeriod block

* fix to return BurnBasePool params

* allow to change oracle feeder to other address

* return error when oracle prevote request input is wrong

* remove unnecessary storage access at oracle tally

* sort price ballot at creation time to reduce duplicate sort cost

* fix testcase

* force to spend gas for failed address conversion

* fix to use ValidateFeeder to properly check bonding status of validator at oracle voting

* remove unnecessary oracle price ballot sort

* prevent empty treasury epoch state stored for cumulateive epochs

* prevent to create multiple vesting schedules for a denom

* remove CodeID from the MsgStoreCode

* fix simulation

docs/core/proto-docs.md

@@ -13654,7 +13654,6 @@ Wasm code to the system
 | ----- | ---- | ----- | ----------- |
 | `sender` | [string](#string) |  | Sender is the that actor that signed the messages |
 | `wasm_byte_code` | [bytes](#bytes) |  | WASMByteCode can be raw or gzip compressed |
-| `code_id` | [uint64](#uint64) |  | CodeID is optional |
 
 
 

proto/terra/wasm/v1beta1/tx.proto

@@ -34,8 +34,6 @@ message MsgStoreCode {
   string sender = 1 [(gogoproto.moretags) = "yaml:\"sender\""];
   // WASMByteCode can be raw or gzip compressed
   bytes wasm_byte_code = 2 [(gogoproto.moretags) = "yaml:\"wasm_byte_code\"", (gogoproto.customname) = "WASMByteCode"];
-  // CodeID is optional
-  uint64 code_id = 3 [(gogoproto.moretags) = "yaml:\"code_id\"", (gogoproto.customname) = "CodeID"];
 }
 
 // MsgStoreCodeResponse defines the Msg/StoreCode response type.

x/market/keeper/keeper_test.go

@@ -67,6 +67,6 @@ func TestReplenishPools(t *testing.T) {
 
 	burnPoolDelta = input.MarketKeeper.GetBurnPoolDelta(input.Ctx)
 	burnReplenishAmt := burnDiff.QuoInt64(recoveryPeriod)
-	expectedBurnDelta := mintDiff.Sub(burnReplenishAmt)
+	expectedBurnDelta := burnDiff.Sub(burnReplenishAmt)
 	require.Equal(t, expectedBurnDelta, burnPoolDelta)
 }

x/market/keeper/params.go

@@ -14,7 +14,7 @@ func (k Keeper) MintBasePool(ctx sdk.Context) (res sdk.Dec) {
 
 // BurnBasePool is burn liquidity pool(usdr unit) which will be made available per PoolRecoveryPeriod
 func (k Keeper) BurnBasePool(ctx sdk.Context) (res sdk.Dec) {
-	k.paramSpace.Get(ctx, types.KeyMintBasePool, &res)
+	k.paramSpace.Get(ctx, types.KeyBurnBasePool, &res)
 	return
 }
 

x/oracle/abci.go

@@ -16,89 +16,95 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
 	defer telemetry.ModuleMeasureSince(types.ModuleName, time.Now(), telemetry.MetricKeyEndBlocker)
 
 	params := k.GetParams(ctx)
+	if core.IsPeriodLastBlock(ctx, params.VotePeriod) {
 
-	// Not yet time for a tally
-	if !core.IsPeriodLastBlock(ctx, params.VotePeriod) {
-		return
-	}
-
-	// Build claim map over all validators in active set
-	validatorClaimMap := make(map[string]types.Claim)
+		// Build claim map over all validators in active set
+		validatorClaimMap := make(map[string]types.Claim)
 
-	maxValidators := k.StakingKeeper.MaxValidators(ctx)
-	iterator := k.StakingKeeper.ValidatorsPowerStoreIterator(ctx)
-	defer iterator.Close()
+		maxValidators := k.StakingKeeper.MaxValidators(ctx)
+		iterator := k.StakingKeeper.ValidatorsPowerStoreIterator(ctx)
+		defer iterator.Close()
 
-	powerReduction := k.StakingKeeper.PowerReduction(ctx)
+		powerReduction := k.StakingKeeper.PowerReduction(ctx)
 
-	i := 0
-	for ; iterator.Valid() && i < int(maxValidators); iterator.Next() {
-		validator := k.StakingKeeper.Validator(ctx, iterator.Value())
+		i := 0
+		for ; iterator.Valid() && i < int(maxValidators); iterator.Next() {
+			validator := k.StakingKeeper.Validator(ctx, iterator.Value())
 
-		// Exclude not bonded validator
-		if validator.IsBonded() {
-			valAddr := validator.GetOperator()
-			validatorClaimMap[valAddr.String()] = types.NewClaim(validator.GetConsensusPower(powerReduction), 0, 0, valAddr)
-			i++
+			// Exclude not bonded validator
+			if validator.IsBonded() {
+				valAddr := validator.GetOperator()
+				validatorClaimMap[valAddr.String()] = types.NewClaim(validator.GetConsensusPower(powerReduction), 0, 0, valAddr)
+				i++
+			}
 		}
-	}
 
-	// Denom-TobinTax map
-	voteTargets := make(map[string]sdk.Dec)
-	k.IterateTobinTaxes(ctx, func(denom string, tobinTax sdk.Dec) bool {
-		voteTargets[denom] = tobinTax
-		return false
-	})
-
-	// Clear all exchange rates
-	k.IterateLunaExchangeRates(ctx, func(denom string, _ sdk.Dec) (stop bool) {
-		k.DeleteLunaExchangeRate(ctx, denom)
-		return false
-	})
-
-	// Organize votes to ballot by denom
-	// NOTE: **Filter out inactive or jailed validators**
-	// NOTE: **Make abstain votes to have zero vote power**
-	voteMap := k.OrganizeBallotByDenom(ctx, validatorClaimMap)
-
-	if referenceTerra := pickReferenceTerra(ctx, k, voteTargets, voteMap); referenceTerra != "" {
-		// make voteMap of Reference Terra to calculate cross exchange rates
-		ballotRT := voteMap[referenceTerra]
-		voteMapRT := ballotRT.ToMap()
-		exchangeRateRT := ballotRT.WeightedMedian()
-
-		// Iterate through ballots and update exchange rates; drop if not enough votes have been achieved.
-		for denom, ballot := range voteMap {
-
-			// Convert ballot to cross exchange rates
-			if denom != referenceTerra {
-				ballot = ballot.ToCrossRate(voteMapRT)
+		// Denom-TobinTax map
+		voteTargets := make(map[string]sdk.Dec)
+		k.IterateTobinTaxes(ctx, func(denom string, tobinTax sdk.Dec) bool {
+			voteTargets[denom] = tobinTax
+			return false
+		})
+
+		// Clear all exchange rates
+		k.IterateLunaExchangeRates(ctx, func(denom string, _ sdk.Dec) (stop bool) {
+			k.DeleteLunaExchangeRate(ctx, denom)
+			return false
+		})
+
+		// Organize votes to ballot by denom
+		// NOTE: **Filter out inactive or jailed validators**
+		// NOTE: **Make abstain votes to have zero vote power**
+		voteMap := k.OrganizeBallotByDenom(ctx, validatorClaimMap)
+
+		if referenceTerra := pickReferenceTerra(ctx, k, voteTargets, voteMap); referenceTerra != "" {
+			// make voteMap of Reference Terra to calculate cross exchange rates
+			ballotRT := voteMap[referenceTerra]
+			voteMapRT := ballotRT.ToMap()
+			exchangeRateRT := ballotRT.WeightedMedian()
+
+			// Iterate through ballots and update exchange rates; drop if not enough votes have been achieved.
+			for denom, ballot := range voteMap {
+
+				// Convert ballot to cross exchange rates
+				if denom != referenceTerra {
+					ballot = ballot.ToCrossRate(voteMapRT)
+				}
+
+				// Get weighted median of cross exchange rates
+				exchangeRate := Tally(ctx, ballot, params.RewardBand, validatorClaimMap)
+
+				// Transform into the original form uluna/stablecoin
+				if denom != referenceTerra {
+					exchangeRate = exchangeRateRT.Quo(exchangeRate)
+				}
+
+				// Set the exchange rate, emit ABCI event
+				k.SetLunaExchangeRateWithEvent(ctx, denom, exchangeRate)
 			}
+		}
 
-			// Get weighted median of cross exchange rates
-			exchangeRate := Tally(ctx, ballot, params.RewardBand, validatorClaimMap)
-
-			// Transform into the original form uluna/stablecoin
-			if denom != referenceTerra {
-				exchangeRate = exchangeRateRT.Quo(exchangeRate)
+		//---------------------------
+		// Do miss counting & slashing
+		voteTargetsLen := len(voteTargets)
+		for _, claim := range validatorClaimMap {
+			// Skip abstain & valid voters
+			if int(claim.WinCount) == voteTargetsLen {
+				continue
 			}
 
-			// Set the exchange rate, emit ABCI event
-			k.SetLunaExchangeRateWithEvent(ctx, denom, exchangeRate)
+			// Increase miss counter
+			k.SetMissCounter(ctx, claim.Recipient, k.GetMissCounter(ctx, claim.Recipient)+1)
 		}
-	}
 
-	//---------------------------
-	// Do miss counting & slashing
-	voteTargetsLen := len(voteTargets)
-	for _, claim := range validatorClaimMap {
-		// Skip abstain & valid voters
-		if int(claim.WinCount) == voteTargetsLen {
-			continue
-		}
+		// Distribute rewards to ballot winners
+		k.RewardBallotWinners(ctx, validatorClaimMap)
+
+		// Clear the ballot
+		k.ClearBallots(ctx, params.VotePeriod)
 
-		// Increase miss counter
-		k.SetMissCounter(ctx, claim.Recipient, k.GetMissCounter(ctx, claim.Recipient)+1)
+		// Update vote targets and tobin tax
+		k.ApplyWhitelist(ctx, params.Whitelist, voteTargets)
 	}
 
 	// Do slash who did miss voting over threshold and
@@ -107,14 +113,5 @@ func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
 		k.SlashAndResetMissCounters(ctx)
 	}
 
-	// Distribute rewards to ballot winners
-	k.RewardBallotWinners(ctx, validatorClaimMap)
-
-	// Clear the ballot
-	k.ClearBallots(ctx, params.VotePeriod)
-
-	// Update vote targets and tobin tax
-	k.ApplyWhitelist(ctx, params.Whitelist, voteTargets)
-
 	return
 }

x/oracle/abci_test.go

@@ -3,6 +3,7 @@ package oracle_test
 import (
 	"fmt"
 	"math"
+	"sort"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -167,6 +168,7 @@ func TestOracleTally(t *testing.T) {
 			Recipient: valAddr,
 		}
 	}
+	sort.Sort(ballot)
 	weightedMedian := ballot.WeightedMedian()
 	standardDeviation := ballot.StandardDeviation()
 	maxSpread := weightedMedian.Mul(input.OracleKeeper.RewardBand(input.Ctx).QuoInt64(2))

x/oracle/client/rest/tx.go

@@ -59,13 +59,6 @@ func newDelegateHandlerFunction(clientCtx client.Context) http.HandlerFunc {
 			return
 		}
 
-		// Bytes comparison, so do not require type conversion
-		if !voterAddr.Equals(req.Feeder) {
-			err := fmt.Errorf("[%v] can not change [%v] delegation", req.Feeder, voterAddr)
-			rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
-			return
-		}
-
 		// create the message
 		msg := types.NewMsgDelegateFeedConsent(voterAddr, req.Feeder)
 		if rest.CheckBadRequestError(w, msg.ValidateBasic()) {
@@ -109,12 +102,15 @@ func newAggregatePrevoteHandlerFunction(clientCtx client.Context) http.HandlerFu
 			}
 
 			hash = types.GetAggregateVoteHash(req.Salt, req.ExchangeRates, voterAddr)
-		} else {
+		} else if len(req.Hash) > 0 {
 			hash, err = types.AggregateVoteHashFromHexString(req.Hash)
 			if err != nil {
 				rest.WriteErrorResponse(w, http.StatusBadRequest, err.Error())
 				return
 			}
+		} else {
+			rest.WriteErrorResponse(w, http.StatusBadRequest, "must provide Hash or (ExchangeRates & Salt)")
+			return
 		}
 
 		// create the message

x/oracle/keeper/ballot.go

@@ -2,6 +2,7 @@ package keeper
 
 import (
 	"fmt"
+	"sort"
 	"strings"
 
 	"github.com/terra-money/core/x/oracle/types"
@@ -44,6 +45,13 @@ func (k Keeper) OrganizeBallotByDenom(ctx sdk.Context, validatorClaimMap map[str
 	}
 
 	k.IterateAggregateExchangeRateVotes(ctx, aggregateHandler)
+
+	// sort created ballot
+	for denom, ballot := range votes {
+		sort.Sort(ballot)
+		votes[denom] = ballot
+	}
+
 	return
 }
 

x/oracle/keeper/keeper.go

@@ -358,7 +358,7 @@ func (k Keeper) ValidateFeeder(ctx sdk.Context, feederAddr sdk.AccAddress, valid
 	}
 
 	// Check that the given validator exists
-	if val := k.StakingKeeper.Validator(ctx, validatorAddr); !val.IsBonded() {
+	if val := k.StakingKeeper.Validator(ctx, validatorAddr); val == nil || !val.IsBonded() {
 		return sdkerrors.Wrapf(stakingtypes.ErrNoValidatorFound, "validator %s is not active set", validatorAddr.String())
 	}
 

x/oracle/keeper/msg_server.go

@@ -33,11 +33,8 @@ func (ms msgServer) AggregateExchangeRatePrevote(goCtx context.Context, msg *typ
 		return nil, err
 	}
 
-	if !feederAddr.Equals(valAddr) {
-		delegate := ms.GetFeederDelegation(ctx, valAddr)
-		if !delegate.Equals(feederAddr) {
-			return nil, sdkerrors.Wrap(types.ErrNoVotingPermission, msg.Feeder)
-		}
+	if err := ms.ValidateFeeder(ctx, feederAddr, valAddr); err != nil {
+		return nil, err
 	}
 
 	// Convert hex string to votehash
@@ -46,12 +43,6 @@ func (ms msgServer) AggregateExchangeRatePrevote(goCtx context.Context, msg *typ
 		return nil, sdkerrors.Wrap(types.ErrInvalidHash, err.Error())
 	}
 
-	// Check that the given validator exists
-	val := ms.StakingKeeper.Validator(ctx, valAddr)
-	if val == nil {
-		return nil, sdkerrors.Wrap(stakingtypes.ErrNoValidatorFound, msg.Validator)
-	}
-
 	aggregatePrevote := types.NewAggregateExchangeRatePrevote(voteHash, valAddr, uint64(ctx.BlockHeight()))
 	ms.SetAggregateExchangeRatePrevote(ctx, valAddr, aggregatePrevote)
 
@@ -83,17 +74,8 @@ func (ms msgServer) AggregateExchangeRateVote(goCtx context.Context, msg *types.
 		return nil, err
 	}
 
-	if !feederAddr.Equals(valAddr) {
-		delegate := ms.GetFeederDelegation(ctx, valAddr)
-		if !delegate.Equals(feederAddr) {
-			return nil, sdkerrors.Wrap(types.ErrNoVotingPermission, msg.Feeder)
-		}
-	}
-
-	// Check that the given validator exists
-	val := ms.StakingKeeper.Validator(ctx, valAddr)
-	if val == nil {
-		return nil, sdkerrors.Wrap(stakingtypes.ErrNoValidatorFound, msg.Validator)
+	if err := ms.ValidateFeeder(ctx, feederAddr, valAddr); err != nil {
+		return nil, err
 	}
 
 	params := ms.GetParams(ctx)

x/oracle/simulation/operations.go

@@ -91,7 +91,7 @@ func SimulateMsgAggregateExchangeRatePrevote(ak types.AccountKeeper, bk types.Ba
 
 		// ensure the validator exists
 		val := k.StakingKeeper.Validator(ctx, address)
-		if val == nil {
+		if val == nil || !val.IsBonded() {
 			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgAggregateExchangeRatePrevote, "unable to find validator"), nil, nil
 		}
 
@@ -155,7 +155,7 @@ func SimulateMsgAggregateExchangeRateVote(ak types.AccountKeeper, bk types.BankK
 
 		// ensure the validator exists
 		val := k.StakingKeeper.Validator(ctx, address)
-		if val == nil {
+		if val == nil || !val.IsBonded() {
 			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgAggregateExchangeRateVote, "unable to find validator"), nil, nil
 		}
 

x/oracle/tally.go

@@ -42,10 +42,7 @@ func Tally(ctx sdk.Context, pb types.ExchangeRateBallot, rewardBand sdk.Dec, val
 }
 
 // ballot for the asset is passing the threshold amount of voting power
-func ballotIsPassing(ctx sdk.Context, ballot types.ExchangeRateBallot, k keeper.Keeper) (sdk.Int, bool) {
-	totalBondedPower := sdk.TokensToConsensusPower(k.StakingKeeper.TotalBondedTokens(ctx), k.StakingKeeper.PowerReduction(ctx))
-	voteThreshold := k.VoteThreshold(ctx)
-	thresholdVotes := voteThreshold.MulInt64(totalBondedPower).RoundInt()
+func ballotIsPassing(ballot types.ExchangeRateBallot, thresholdVotes sdk.Int) (sdk.Int, bool) {
 	ballotPower := sdk.NewInt(ballot.Power())
 	return ballotPower, !ballotPower.IsZero() && ballotPower.GTE(thresholdVotes)
 }
@@ -57,6 +54,10 @@ func pickReferenceTerra(ctx sdk.Context, k keeper.Keeper, voteTargets map[string
 	largestBallotPower := int64(0)
 	referenceTerra := ""
 
+	totalBondedPower := sdk.TokensToConsensusPower(k.StakingKeeper.TotalBondedTokens(ctx), k.StakingKeeper.PowerReduction(ctx))
+	voteThreshold := k.VoteThreshold(ctx)
+	thresholdVotes := voteThreshold.MulInt64(totalBondedPower).RoundInt()
+
 	for denom, ballot := range voteMap {
 		// If denom is not in the voteTargets, or the ballot for it has failed, then skip
 		// and remove it from voteMap for iteration efficiency
@@ -69,7 +70,7 @@ func pickReferenceTerra(ctx sdk.Context, k keeper.Keeper, voteTargets map[string
 
 		// If the ballot is not passed, remove it from the voteTargets array
 		// to prevent slashing validators who did valid vote.
-		if power, ok := ballotIsPassing(ctx, ballot, k); ok {
+		if power, ok := ballotIsPassing(ballot, thresholdVotes); ok {
 			ballotPower = power.Int64()
 		} else {
 			delete(voteTargets, denom)