Skip to content

Update price feeds early if part of a batch #905

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

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Conversation

jayantk
Copy link
Contributor

@jayantk jayantk commented Jun 21, 2023

I was looking at the sui pusher's txs and I noticed that it currently only updates a subset of the price feeds in the pushed VAA. This behavior is an interaction between the structure of the sui contract (with the hot potato etc) and the price pusher.

I'm fixing this problem by adding a feature we'll want anyway, which is to piggyback additional price updates on to a push. With accumulators, it's very cheap to add an additional update to an existing price push, so we'll want to take advantage of this by pushing more feeds than just the ones that met the triggering conditions.

This PR adds an extra set of triggering conditions for an early update. If at least one feed meets the normal triggering conditions, we'll push a batch of updates that includes the original update + all feeds meeting the early update conditions. Using this, we can also set early_update.time_difference = 0 in the sui config to solve the specific problem as well.

I haven't been able to run this since testnet is down, but i will make sure it works before merging.

@vercel
Copy link

vercel bot commented Jun 21, 2023

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

2 Ignored Deployments
Name Status Preview Updated (UTC)
example-oracle-amm ⬜️ Ignored (Inspect) Jun 21, 2023 4:22pm
xc-admin-frontend ⬜️ Ignored (Inspect) Jun 21, 2023 4:22pm

`Pushing updates from wallet address: ${pythContractFactory
.createWeb3PayerProvider()
.getAddress()}`
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

drive-by for ease of use

Copy link
Collaborator

@ali-behjati ali-behjati left a comment

Choose a reason for hiding this comment

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

lgtm! :shipit:

@jayantk jayantk merged commit 339b2f6 into main Jun 21, 2023
@jayantk jayantk deleted the price_pusher_batch branch June 21, 2023 21:07
const priceId = priceConfig.id;

// There is no price to update the target with.
if (sourceLatestPrice === undefined) {
return false;
return UpdateCondition.YES;

Choose a reason for hiding this comment

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

is this a bug ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes unfortunately. fix is in #2730

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.

3 participants