proposal(twap): consider using last spot price instead of zero on error in getSpotPrices

[p0mvn]

Background

Context:

osmosis/x/twap/logic.go

Lines 46 to 54 in 4bbd3fb

	// In the event of an error, we just sanity replace empty values with zero values
	// so that the numbers can be still be calculated within TWAPs over error values
	// TODO: Should we be using the last spot price?
	if (sp0 == sdk.Dec{}) {
	sp0 = sdk.ZeroDec()
	}
	if (sp1 == sdk.Dec{}) {
	sp1 = sdk.ZeroDec()
	}

I think this is a worthwhile improvement to make before v12.

Acceptance Criteria

• decide if including in v12

[AlpinYukseloglu]

I agree with switching to last spot price as this causes TWAPs to level out and stay stable after an error, which is the property that I believe we wanted to protect by leaving TWAPs open after spot price errors

[mattverse]

Let me think about the possible edge case here, is it going to error when the pool does not have the last spot price? By last spot price, are we referring to the spot price recorded in the previous epoch?

[p0mvn]

Let me think about the possible edge case here, is it going to error when the pool does not have the last spot price? By last spot price, are we referring to the spot price recorded in the previous epoch?

Assume that we have a twap record at time t. Then, at t + 1 we have a swap so a new record needs to be created with the updated spot prices. Let us get a spot price error.

Currently, we would set the spot price to 0 at t + 1 since it errored. The problem with that is that it negatively impacts the calculations by diverging them significantly from the true value.

Instead of setting it to zero, we might want to reuse the spot price from the previous record (at time ``t`)

One edge case I can think of is when we get an error but there is no older record.

I don't think it should be hard to implement the change. I will try getting it out before Monday to further discuss

[mattverse]

Currently, we would set the spot price to 0 at t + 1 since it errored. The problem with that is that it negatively impacts the calculations by diverging them significantly from the true value

If this is the case, I'm definitely for what this issue is proposing! (I wonder if we have a test case showing that rn)

[p0mvn]

Decided on the call that this is not v12 blocking but good to have in the future releases