Skip to content

Conversation

@0xfirefist
Copy link
Contributor

@0xfirefist 0xfirefist commented Feb 23, 2023

moved shouldUpdate to price-config as it is closely related to it
pusher changed to controller + moved the code to push updates into evm.ts
evm.ts contains everything related to evm. PriceGetter, PricePusher, Contract

one change i do think should have been in the separate PR in this file price_pusher/src/pyth-price-listener.ts

@vercel
Copy link

vercel bot commented Feb 23, 2023

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

2 Ignored Deployments
Name Status Preview Updated
example-oracle-amm ⬜️ Ignored (Inspect) Feb 23, 2023 at 5:10PM (UTC)
xc-admin-frontend ⬜️ Ignored (Inspect) Feb 23, 2023 at 5:10PM (UTC)

@0xfirefist 0xfirefist requested a review from jayantk February 23, 2023 17:11
this.targetChainPricePusher.updatePriceFeed(priceIds, pubTimesToPush);
await sleep(this.cooldownDuration * 1000);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

probably a good idea to have a stop() function on this class also. It makes it easier to reuse in code (vs. as a cli)

if (priceIds.length !== pubTimesToPush.length)
throw new Error("Invalid arguments");

const priceIdsWith0x = priceIds.map((priceId) => addLeading0x(priceId));
Copy link
Contributor

Choose a reason for hiding this comment

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

in the future, we should really make a class for price ids so we can expose methods like getHexStringWithLeadingZero or whatever

@0xfirefist 0xfirefist changed the title Refactor Price Pusher to support other chains [price-pusher] Refactor Price Pusher to support other chains Feb 24, 2023
@0xfirefist
Copy link
Contributor Author

I will send other PRs for above changes!

@0xfirefist 0xfirefist merged commit ae88640 into main Feb 24, 2023
@0xfirefist 0xfirefist deleted the refactor-price-pusher branch February 24, 2023 12:01
return priceConfig;
});
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you move it here? I think maybe it's better to be on controller.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is very closely related to price config.
if config changes we will need to update the should Update logic too.
controller doesn't need to know anything related to priceConfig.
I am thinking in this direction.

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.

4 participants