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

Eth price syncer using Chainlink oracle #731

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Eth price syncer using Chainlink oracle #731

wants to merge 10 commits into from

Conversation

luis-herasme
Copy link
Member

@luis-herasme luis-herasme commented Feb 24, 2025

Checklist

  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

Adds package to fetch the ETH price at a given timestamp.
We need this to complete #685

Related Issue (Optional)

#617

Copy link

changeset-bot bot commented Feb 24, 2025

⚠️ No Changeset found

Latest commit: 89c17b0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Feb 24, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
blobscan-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 1:26pm
blobscan-staging ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 1:26pm
4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
blobscan-gnosis ⬜️ Ignored (Inspect) Visit Preview Feb 24, 2025 1:26pm
blobscan-holesky ⬜️ Ignored (Inspect) Visit Preview Feb 24, 2025 1:26pm
blobscan-mainnet ⬜️ Ignored (Inspect) Visit Preview Feb 24, 2025 1:26pm
blobscan-sepolia ⬜️ Ignored (Inspect) Visit Preview Feb 24, 2025 1:26pm

Copy link
Contributor

github-actions bot commented Feb 24, 2025

📦 Next.js Bundle Analysis for @blobscan/web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

This PR introduced no changes to the JavaScript bundle! 🙌

Copy link
Member

@PabloCastellano PabloCastellano left a comment

Choose a reason for hiding this comment

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

Nice to see this implemented! I've left you some suggestions and a request to improve the documentation for this PR.

@@ -0,0 +1,8 @@
-- CreateTable
CREATE TABLE "eth_price" (
Copy link
Member

Choose a reason for hiding this comment

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

I assume we are going to save the price in USD, right? Then let's better rename it:

Suggested change
CREATE TABLE "eth_price" (
CREATE TABLE "ethusd_price" (

@@ -301,6 +301,14 @@ model OverallStats {
@@map("overall_stats")
}

model ETHPrice {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
model ETHPrice {
model EthUsdPrice {

},
});

this.logger.info(`Fetched price data: ${JSON.stringify(priceData)}`);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.logger.info(`Fetched price data: ${JSON.stringify(priceData)}`);
this.logger.info(`Fetched price data: ${JSON.stringify(priceData)} USD`);

@@ -60,6 +60,7 @@ nextjs:
| `WEAVEVM_API_KEY` | API key required to authenticate requests to the WeaveVM blob storage reference updater endpoint | No | (empty) |
| `STATS_SYNCER_DAILY_CRON_PATTERN` | Cron pattern for the daily stats job | No | `30 0 * * * *` |
| `STATS_SYNCER_OVERALL_CRON_PATTERN` | Cron pattern for the overall stats job | No | `*/15 * * * *` |
| `ETH_PRICE_SYNCER_CRON_PATTERN` | Cron pattern for the ETH price job | No | `0 * * * *` |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| `ETH_PRICE_SYNCER_CRON_PATTERN` | Cron pattern for the ETH price job | No | `0 * * * *` |
| `ETH_PRICE_SYNCER_CRON_PATTERN` | Cron pattern for the job that periodically stores ETH price in database | No | `0 * * * *` |

@@ -83,6 +83,9 @@ export const env = createEnv({
.url()
.default("http://localhost:4318"),

// ETH Price
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ETH Price
// ETH Price (default: every minute)

...mainnet,
rpcUrls: {
default: {
http: [process.env.RPC_URL!],
Copy link
Member

Choose a reason for hiding this comment

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

Is this a new variable? If so, remember adding it to the documentation

});

if (response === null) {
throw new Error(`No data found for target timestamp: ${targetTimestamp}`);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
throw new Error(`No data found for target timestamp: ${targetTimestamp}`);
throw new Error(`Could not retrieve ETH price from Chainlink oracle using timestamp: ${targetTimestamp}`);

* @param tolerance The maximum difference between the target timestamp and the timestamp of the data.
* @returns The price of the asset at the given timestamp.
**/
export async function getPriceByTimestamp({
Copy link
Member

Choose a reason for hiding this comment

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

Could you improve this documentation? What I really miss from this PR is a comment that explains the process that you've implemented for querying an oracle using a timestamp. Also explaining the relation with other functions (such as getPhaseAggregators or getClosestRoundData) and adding a link to some external documentation to learn more about the protocol would be helpful.

@PabloCastellano PabloCastellano changed the title Eth price Eth price syncer Mar 3, 2025
@PabloCastellano PabloCastellano changed the title Eth price syncer Eth price syncer using Chainlink oracle Mar 3, 2025
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.

2 participants