fix: x/oracle end_blocker panic

[kosegor]

This PR contains a fix for the chain halting during the EndBlocker function. The scenario to reproduce it is as follows:

• Start a fresh chain with median_stamp_period < historic_stamp_period.
• Populate x/leverage registry with tokens and wait until x/oracle EndBlocker executes.
• A panic will raise halting the chain with the following error: panic: denom: ETH: empty price list passed in

[codecov]

Codecov Report

Merging #2185 (2f316d4) into main (7f05ad4) will decrease coverage by 6.61%.
Report is 170 commits behind head on main.
The diff coverage is 81.67%.

@@            Coverage Diff             @@
##             main    #2185      +/-   ##
==========================================
- Coverage   75.38%   68.78%   -6.61%     
==========================================
  Files         100      157      +57     
  Lines        8025    11730    +3705     
==========================================
+ Hits         6050     8068    +2018     
- Misses       1589     3120    +1531     
- Partials      386      542     +156     

Files Changed	Coverage Δ
ante/spam_prevention.go	75.92% <ø> (ø)
util/coin/coin.go	12.50% <0.00%> (ø)
x/incentive/codec.go	47.82% <ø> (+9.89%)	⬆️
x/incentive/keeper/invariants.go	0.00% <0.00%> (ø)
x/incentive/keeper/store.go	70.67% <ø> (ø)
x/incentive/keeper/unbonding.go	80.45% <ø> (ø)
x/incentive/keeper/update.go	52.11% <ø> (ø)
x/incentive/keys.go	100.00% <ø> (ø)
x/incentive/msgs.go	77.41% <ø> (-0.81%)	⬇️
x/incentive/params.go	89.28% <ø> (-10.72%)	⬇️
... and 58 more

... and 60 files with indirect coverage changes

[adamewozniak]

This doesn’t look like a bug in endblocker, rather a condition we shouldn’t expect to be in the genesis file. Imo we shouldn’t merge this in

[toteki]

Maybe we can fix one layer lower in the functions - missing medians shouldn't be a chain halt, just like missing prices aren't.

We could do:

func (k Keeper) CalcAndSetHistoricMedian(
	ctx sdk.Context,
	denom string,
) error {
	historicPrices := k.historicPrices(ctx, denom, k.MaximumPriceStamps(ctx))
	if len(historicPrices) == 0 {
		return nil
	}
	median, err := decmath.Median(historicPrices)
	if err != nil {
		return errors.Wrap(err, "denom: "+denom)
	}

	block := uint64(ctx.BlockHeight())
	k.SetHistoricMedian(ctx, denom, block, median)
	return k.calcAndSetHistoricMedianDeviation(ctx, denom, median, historicPrices)
}

(the return nil and its if statement are the only changes)

@adamewozniak

[adamewozniak]

Maybe we can fix one layer lower in the functions - missing medians shouldn't be a chain halt, just like missing prices aren't.

We could do:

func (k Keeper) CalcAndSetHistoricMedian(

	ctx sdk.Context,

	denom string,

) error {

	historicPrices := k.historicPrices(ctx, denom, k.MaximumPriceStamps(ctx))

	if len(historicPrices) == 0 {

		return nil

	}

	median, err := decmath.Median(historicPrices)

	if err != nil {

		return errors.Wrap(err, "denom: "+denom)

	}



	block := uint64(ctx.BlockHeight())

	k.SetHistoricMedian(ctx, denom, block, median)

	return k.calcAndSetHistoricMedianDeviation(ctx, denom, median, historicPrices)

}

(the return nil and its if statement are the only changes)

@adamewozniak

Seems like a much more appropriate fix imo. Maybe a comment / log in this function that "this should never happen"?

[kosegor]

I applied @toteki solution. @adamewozniak check it out.

[adamewozniak]

Lgtm, one nit