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

feat: updated history module #353

Merged
merged 12 commits into from
Mar 11, 2022
Merged

Conversation

MonikaCat
Copy link
Contributor

Description

Closes: #348

  • Updated periodic ops to query token price and store it inside token_price table every 2 mins
  • Added periodic ops to history module to query historic token price and store it inside token_price_history table every 1hr

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch
  • provided a link to the relevant issue or specification
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@MonikaCat MonikaCat added the automerge Automatically merge PR once all prerequisites pass label Mar 9, 2022
@RiccardoM
Copy link
Contributor

What is the current history module storing since we replaced all historic data with Hasura actions? Cause I think it might be easier to move the historic token storage inside the pricefeed module itself so one module does everything there's needed rather than splitting into another module (which needs to be enabled)

@MonikaCat
Copy link
Contributor Author

Currently history module is storing historical price data only. Sure, I moved it inside pricefeed module as per you suggestion 👍🏻

@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2022

Codecov Report

❗ No coverage uploaded for pull request base (v2/cosmos/stargate@29a4433). Click here to learn what that means.
The diff coverage is n/a.

@@                  Coverage Diff                  @@
##             v2/cosmos/stargate     #353   +/-   ##
=====================================================
  Coverage                      ?   57.58%           
=====================================================
  Files                         ?       17           
  Lines                         ?     1339           
  Branches                      ?        0           
=====================================================
  Hits                          ?      771           
  Misses                        ?      472           
  Partials                      ?       96           

Str("operation", "pricefeed").
Msg("updating token price and market cap history")

prices, err := m.getTokenPrices()
Copy link
Contributor

Choose a reason for hiding this comment

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

I removed the part that forced all the prices to have LastUpdated set to time.Now() as I think it wasn't correct. @MonikaCat Was there a specific reason that one was there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RiccardoM Last Updated timestamp is fetched from the coingecko and it reflects the time when the price was last updated. We need the exact value at exact time every 1 hour to plot the graph. If price hasn't change within that 1 hour, the returned timestamp will be the same as one hour ago and it will not be stored in db as it will be a duplicated value. So that's why we should pass the current timestamp rather than the timestamp fetched from coingecko.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok thanks, added back with a comment

@mergify mergify bot merged commit 081b40f into v2/cosmos/stargate Mar 11, 2022
@mergify mergify bot deleted the monika/update-history-module branch March 11, 2022 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatically merge PR once all prerequisites pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable Token Price History without History Module
4 participants