Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(twap/e2e): set geometric accum to zero; reproduce testnet bug #3972

Merged
merged 3 commits into from
Jan 10, 2023

Conversation

p0mvn
Copy link
Member

@p0mvn p0mvn commented Jan 10, 2023

Closes: #XXX

What is the purpose of the change

No migration alternative to: #3970

In #3542, it was concluded that we do not need to perform the migrations to initialize the geometric twap accumulator.

However, the testnet halted due to the geometric twap accumulator ending up uninitialized.

The reason why our e2e test did not catch this problem was because we never swapped against a pool that was created pre-upgrade after the upgrade itself.

This PR reproduced the testnet halt bug and fixed it by forcing to set the geometric twap accumulator to zero after it is parsed.

Brief Changelog

  • reproduce testnet halt
  • reset the accumulator to zero
  • test more thoroughly and document

Testing and Verifying

This change added e2e and ParseRecordFromBz tests.

Brief Changelog

Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

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

@github-actions github-actions bot added the C:x/twap Changes to the twap module label Jan 10, 2023
@p0mvn p0mvn added V:state/breaking State machine breaking PR A:backport/v14.x backport patches to v14.x branch labels Jan 10, 2023
@p0mvn p0mvn marked this pull request as ready for review January 10, 2023 14:55
Copy link
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.

This LGTM

@p0mvn p0mvn merged commit a693a3b into main Jan 10, 2023
@p0mvn p0mvn deleted the roman/twap-e2e-testnet-halt2 branch January 10, 2023 16:04
mergify bot pushed a commit that referenced this pull request Jan 10, 2023
)

* fix(twap/e2e): set geometric accum to zero; reproduce testnet bug

* test TestParseTwapFromBz

* clean up

(cherry picked from commit a693a3b)
czarcas7ic pushed a commit that referenced this pull request Jan 10, 2023
) (#3974)

* fix(twap/e2e): set geometric accum to zero; reproduce testnet bug

* test TestParseTwapFromBz

* clean up

(cherry picked from commit a693a3b)

Co-authored-by: Roman <roman@osmosis.team>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v14.x backport patches to v14.x branch C:x/twap Changes to the twap module V:state/breaking State machine breaking PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants