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

twap at height NTRN-357 #7

Merged
merged 13 commits into from
Mar 9, 2023
Merged

twap at height NTRN-357 #7

merged 13 commits into from
Mar 9, 2023

Conversation

quasisamurai
Copy link
Contributor

@quasisamurai quasisamurai commented Feb 13, 2023

TASK

This is a patched astroport oracle contract with necessary astroport contracts and also some modifications 4 Neutron needs:

  • Implemented twap_at_height query
  • PERIOD const is now parameter
  • implemented simple and complex tests

since the astroport contract has nothing to integrate with at the moment, the tests are implemented directly in the repository (see integration.rs ) discussed w @swelf19

How to test: you don't need to since you're seeing green check mark in this PR

@quasisamurai quasisamurai marked this pull request as ready for review February 14, 2023 10:17
contracts/astroport/oracle/src/contract.rs Outdated Show resolved Hide resolved
contracts/astroport/oracle/src/state.rs Outdated Show resolved Hide resolved
pr0n00gler
pr0n00gler previously approved these changes Feb 28, 2023
@pr0n00gler pr0n00gler dismissed their stale review March 1, 2023 15:01

need to recheck smth

Copy link
Contributor

@foxpy foxpy left a comment

Choose a reason for hiding this comment

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

code looks fine, though I am not sure how are we going to resolve dependency hell with cosmwasm-std and cw-X libraries.

Copy link

@oldremez oldremez left a comment

Choose a reason for hiding this comment

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

I believe there's a lack of tests with a height between two snapshots. Current tests cover only cases with exactly those blocks at which the oracle was updated. But in a real situation, the queries will be done to the arbitrary block that in general lies between snapshots. And what is the expected behavior here: do we need to return the past TWAP calculated or the next one (since the block we get a query for lies in the period of time for which the next TWAP is calculated, right?).

Also, out of scope but maybe we should have a todo for it: if we keep the history anyway, shouldn't we allow updating at arbitrary blocks? I mean for any block we can find the previous snapshot that is more than the update period far away. Or it will lead to more gas consumption?

@zavgorodnii zavgorodnii merged commit 0a2f9ab into main Mar 9, 2023
@ratik ratik deleted the feat/twap-at-heigh branch March 15, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants