-
Notifications
You must be signed in to change notification settings - Fork 220
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
fix(pyth-solana-receiver): improve perf and security #2222
fix(pyth-solana-receiver): improve perf and security #2222
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
target_chains/solana/pyth_solana_receiver_sdk/src/price_update.rs
Outdated
Show resolved
Hide resolved
|
||
// Ensure the twap window size is as expected | ||
// Allow for +/- 1 second tolerance to account for the imprecision introduced by Solana block times | ||
const TOLERANCE: i64 = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get why you need TOLERANCE
here. There should be at least one VAA per second with 400 milliseconds blocktimes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding was that there's a VAA roughly every 400ms. That means that if I request a 1 second TWAP (t0=1000 and t1=2000), the start VAA might have timestamp 800 and the end VAA might have timestamp 2000. That would result in a 1200ms window. If i had a strict check for window size, it would fail since 1200 != 1000. This is why i added the tolerance... but now i'm noticing that the start_time and end_time precision is in seconds and not milliseconds, so this shouldn't matter! Will go ahead and remove the tolerance. Does this check out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove the +- tolerance, let me know if you disagree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tyvm
Purpose
Performance and security improvements to the SVM TWAP feature.
Implementation details
slot_diff
calculation incalculate_twap
get_twap_no_older_than
function only checked that the twap'send_time
was more recent thanmaximum_age
. However, the window size of the twap wasn't checked. This could result in a scenario where an attacker could frontrun a transaction that consumes a hardcoded TwapUpdate PDA. The attacker could overwrite the account with a verified twap of an unexpected window (very long/short). This could have the effect of manipulating, for example, the LTV ratio calculation of a lending protocol.Testing
Updated SDK test suite