Update record sanity check

x/twap/export_test.go

@@ -44,7 +44,7 @@ func (k Keeper) GetChangedPools(ctx sdk.Context) []uint64 {
 	return k.getChangedPools(ctx)
 }
 
-func (k Keeper) UpdateRecord(ctx sdk.Context, record types.TwapRecord) types.TwapRecord {
+func (k Keeper) UpdateRecord(ctx sdk.Context, record types.TwapRecord) (types.TwapRecord, error) {
 	return k.updateRecord(ctx, record)
 }
 

x/twap/logic.go

@@ -132,15 +132,31 @@ func (k Keeper) updateRecords(ctx sdk.Context, poolId uint64) error {
 	}
 
 	for _, record := range records {
-		newRecord := k.updateRecord(ctx, record)
+		newRecord, err := k.updateRecord(ctx, record)
+		if err != nil {
+			return err
+		}
 		k.storeNewRecord(ctx, newRecord)
 	}
 	return nil
 }
 
 // updateRecord returns a new record with updated accumulators and block time
 // for the current block time.
-func (k Keeper) updateRecord(ctx sdk.Context, record types.TwapRecord) types.TwapRecord {
+func (k Keeper) updateRecord(ctx sdk.Context, record types.TwapRecord) (types.TwapRecord, error) {
+	// Returns error for last updated records in the same block.
+	// Exception: record is initialized when creating a pool,
+	// then the TwapAccumulator variables are zero.
+	if (record.Height >= ctx.BlockHeight() || !record.Time.Before(ctx.BlockTime())) &&
+		!record.P1ArithmeticTwapAccumulator.Equal(sdk.ZeroDec()) &&
+		!record.P0ArithmeticTwapAccumulator.Equal(sdk.ZeroDec()) {
+		return types.TwapRecord{}, types.InvalidRecordTimeError{
+			RecordBlockHeight: record.Height,
+			RecordTime:        record.Time,
+			ActualBlockHeight: ctx.BlockHeight(),
+			ActualTime:        ctx.BlockTime(),
+		}
+	}
 	newRecord := recordWithUpdatedAccumulators(record, ctx.BlockTime())
 	newRecord.Height = ctx.BlockHeight()
 
@@ -152,7 +168,7 @@ func (k Keeper) updateRecord(ctx sdk.Context, record types.TwapRecord) types.Twa
 	newRecord.P1LastSpotPrice = newSp1
 	newRecord.LastErrorTime = lastErrorTime
 
-	return newRecord
+	return newRecord, nil
 }
 
 // pruneRecords prunes twap records that happened earlier than recordHistoryKeepPeriod

x/twap/logic_test.go

@@ -227,7 +227,8 @@ func (s *TestSuite) TestUpdateRecord() {
 				defaultTwoAssetCoins[1].Denom, defaultTwoAssetCoins[0].Denom,
 				test.spotPriceResult1.Sp, test.spotPriceResult1.Err)
 
-			newRecord := s.twapkeeper.UpdateRecord(s.Ctx, test.record)
+			newRecord, err := s.twapkeeper.UpdateRecord(s.Ctx, test.record)
+			s.Require().NoError(err)
 			s.Equal(test.expRecord, newRecord)
 		})
 	}
@@ -1026,25 +1027,11 @@ func (s *TestSuite) TestUpdateRecords() {
 				},
 			},
 
-			expectedHistoricalRecords: []expectedResults{
-				// The original record at t.
-				{
-					spotPriceA: baseRecord.P0LastSpotPrice,
-					spotPriceB: baseRecord.P1LastSpotPrice,
-				},
-				// The new record added.
-				// TODO: it should not be possible to add a record between existing records.
-				// https://github.com/osmosis-labs/osmosis/issues/2686
-				{
-					spotPriceA:   sdk.OneDec(),
-					spotPriceB:   sdk.OneDec().Add(sdk.OneDec()),
-					isMostRecent: true,
-				},
-				// The original record at t + 1.
-				{
-					spotPriceA: tPlus10sp5Record.P0LastSpotPrice,
-					spotPriceB: tPlus10sp5Record.P1LastSpotPrice,
-				},
+			expectError: types.InvalidRecordTimeError{
+				RecordBlockHeight: tPlus10sp5Record.Height,
+				RecordTime:        tPlus10sp5Record.Time,
+				ActualBlockHeight: (baseRecord.Height + 1),
+				ActualTime:        baseRecord.Time.Add(time.Second * 5),
 			},
 		},
 		// TODO: complete multi-asset pool tests:

x/twap/types/errors.go

@@ -62,3 +62,14 @@ type InvalidRecordCountError struct {
 func (e InvalidRecordCountError) Error() string {
 	return fmt.Sprintf("The number of records do not match, expected: %d\n got: %d", e.Expected, e.Actual)
 }
+
+type InvalidRecordTimeError struct {
+	RecordBlockHeight int64
+	RecordTime        time.Time
+	ActualBlockHeight int64
+	ActualTime        time.Time
+}
+
+func (e InvalidRecordTimeError) Error() string {
+	return fmt.Sprintf("Can not update the record, the context time must be greater than record time. Record: block %d at %s\n Actual: block %d at %s\n", e.RecordBlockHeight, e.RecordTime, e.ActualBlockHeight, e.ActualTime)
+}