Skip to content

fix(x/twap): genesis issue blocking edge net set up#5129

Merged
p0mvn merged 3 commits intomainfrom
roman/fix-twap-genesis-issue
May 9, 2023
Merged

fix(x/twap): genesis issue blocking edge net set up#5129
p0mvn merged 3 commits intomainfrom
roman/fix-twap-genesis-issue

Conversation

@p0mvn
Copy link
Copy Markdown
Member

@p0mvn p0mvn commented May 8, 2023

Closes: #5041

What is the purpose of the change

More context: https://osmosis-network.slack.com/archives/C027ELA290B/p1682692206195569

Currently, our validation for TwapRecords in InitGenesis() is stricter than it should be.

If there is a twap error, it expects both "p0" spot price and "p1" spot price to be zero.

However, that is not the guarantee from the core logic:

osmosis/x/twap/logic.go

Lines 53 to 64 in b101d06

if err0 != nil || err1 != nil {
latestErrTime = ctx.BlockTime()
// 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()
}
}

In the core logic above, if "p0" gets a twap error but "p1" does not, "p0" is set to zero but "p1" isn't.

Therefore, we should change our init genesis logic to match the core's constraints.

Brief Changelog

  • make the change
  • add tests

Testing and Verifying

  • Refactored test cases

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no
  • How is the feature or change documented? not applicable

@p0mvn p0mvn added V:state/compatible/backport State machine compatible PR, should be backported A:backport/v15.x backport patches to v15.x branch labels May 8, 2023
@p0mvn p0mvn requested a review from niccoloraspa May 8, 2023 21:01
@github-actions github-actions bot added the C:x/twap Changes to the twap module label May 8, 2023
@p0mvn p0mvn marked this pull request as ready for review May 8, 2023 21:30
@p0mvn p0mvn requested a review from nicolaslara as a code owner May 8, 2023 21:30
Copy link
Copy Markdown
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@p0mvn p0mvn merged commit a131abf into main May 9, 2023
@p0mvn p0mvn deleted the roman/fix-twap-genesis-issue branch May 9, 2023 02:32
mergify bot pushed a commit that referenced this pull request May 9, 2023
* fix(x/twap): genesis issue blocking edge net set up

* changelog

* comment

(cherry picked from commit a131abf)

# Conflicts:
#	CHANGELOG.md
p0mvn added a commit that referenced this pull request May 9, 2023
pysel pushed a commit that referenced this pull request Jun 6, 2023
* fix(x/twap): genesis issue blocking edge net set up

* changelog

* comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A:backport/v15.x backport patches to v15.x branch C:x/twap Changes to the twap module V:state/compatible/backport State machine compatible PR, should be backported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v16 blocker: twap records failing init genesis when state-exported v16 testnet is created from v15 genesis

2 participants