refactor(twap): add sanity check in `updateRecord` to compare `record` time and height against `ctx`

[p0mvn]

Background

This was observed during testing of updateRecords. It is possible to set up a testing environment in such a way as to be able to insert a record between the existing records.

For example, assume a two-asset pool and 2 records at the times t and t + 2. If I set up ctx.BlockTime to be t + 1, I can insert a new record in-between without error.

While this should not be possible to reproduce in real environment, having a sanity check in updateRecord to ensure the following invariants would be useful:

• ctx.BlockTime > record.Time
• ctx.BlockHeight > record.Height

updateRecord and updateRecords should be returning an error in these cases.

Suggested Design

Return error if any of the above invariants are not satisfied.

Acceptance Criteria

• invariants are added to updateRecord
• new code branches of updateRecord are covered by tests
• existing updateRecords tests are refactored / adjusted to cover the new behavior

[hieuvubk]

https://github.com/osmosis-labs/osmosis/blob/1111d84631be02fc3c7e8ebc56a0c56a044fa476/x/twap/listeners_test.go#L26 @p0mvn We have an exception here. With sanity check added, swap on pool creation block should return err. What do u think about that?

[hieuvubk]

https://github.com/osmosis-labs/osmosis/blob/1111d84631be02fc3c7e8ebc56a0c56a044fa476/x/twap/listeners_test.go#L26

@p0mvn We have an exception here. With sanity check added, swap on pool creation block should return err. What do u think about that?

[hieuvubk]

https://github.com/osmosis-labs/osmosis/blob/1111d84631be02fc3c7e8ebc56a0c56a044fa476/x/twap/listeners_test.go#L26

@p0mvn We have an exception here. With sanity check added, swap on pool creation block should return err. What do u think about that?

I think we should discuss with @mattverse owning this commit

[mattverse]

@hieuvubk What causes the error? At a light glance, I don't see anything calling updateRecord with in the afterPoolCreation test

[hieuvubk]

Sr to bother , I checked actually this comes from EndBlock( just logged the err) when updating the record created in the same block in afterCreatePool. So I updated the condition again by checking the case when creating the pool. And should we use twapAccumulator variables to check the record is unedited?

[hieuvubk]

@p0mvn @mattverse Reopend this pr cause its still available for issue. Could u review it? Thanksss