fix(pyth-solana-receiver): improve perf and security (#2222)

* refactor: reuse slots_diff calculation

* fix: add window size check to get_twap_no_older_than

* nit: fix comment

* fix: remove tolerance in window size check

* Update target_chains/solana/pyth_solana_receiver_sdk/src/price_update.rs

Co-authored-by: guibescos <[email protected]>

---------

Co-authored-by: guibescos <[email protected]>

Author: tejasbadadare

target_chains/solana/Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "pyth-solana-receiver"
-version = "0.2.0"
+version = "0.2.1"
 description = "Created with Anchor"
 edition = "2021"
 

target_chains/solana/programs/pyth-solana-receiver/Cargo.toml

@@ -575,18 +575,14 @@ fn calculate_twap(start_msg: &TwapMessage, end_msg: &TwapMessage) -> Result<(i64
 
     // Calculate down_slots_ratio as an integer between 0 and 1_000_000
     // A value of 1_000_000 means all slots were missed and 0 means no slots were missed.
-    let total_slots = end_msg
-        .publish_slot
-        .checked_sub(start_msg.publish_slot)
-        .ok_or(ReceiverError::TwapCalculationOverflow)?;
     let total_down_slots = end_msg
         .num_down_slots
         .checked_sub(start_msg.num_down_slots)
         .ok_or(ReceiverError::TwapCalculationOverflow)?;
     let down_slots_ratio = total_down_slots
         .checked_mul(1_000_000)
         .ok_or(ReceiverError::TwapCalculationOverflow)?
-        .checked_div(total_slots)
+        .checked_div(slot_diff)
         .ok_or(ReceiverError::TwapCalculationOverflow)?;
     // down_slots_ratio is a number in [0, 1_000_000], so we only need 32 unsigned bits
     let down_slots_ratio =

target_chains/solana/programs/pyth-solana-receiver/src/lib.rs

@@ -1,6 +1,6 @@
 [package]
 name = "pyth-solana-receiver-sdk"
-version = "0.4.0"
+version = "0.5.0"
 description = "SDK for the Pyth Solana Receiver program"
 authors = ["Pyth Data Association"]
 repository = "https://github.com/pyth-network/pyth-crosschain"

target_chains/solana/pyth_solana_receiver_sdk/Cargo.toml

@@ -5,6 +5,8 @@ use anchor_lang::error_code;
 pub enum GetPriceError {
     #[msg("This price feed update's age exceeds the requested maximum age")]
     PriceTooOld = 10000, // Big number to avoid conflicts with the SDK user's program error codes
+    #[msg("This TWAP update's window size is invalid")]
+    InvalidWindowSize,
     #[msg("The price feed update doesn't match the requested feed id")]
     MismatchedFeedId,
     #[msg("This price feed update has a lower verification level than the one requested")]

target_chains/solana/pyth_solana_receiver_sdk/src/error.rs

@@ -87,10 +87,11 @@ impl TwapUpdate {
     /// # Warning
     /// This function does not check :
     /// - How recent the price is
+    /// - If the TWAP's window size is expected
     /// - Whether the price update has been verified
     ///
     /// It is therefore unsafe to use this function without any extra checks,
-    /// as it allows for the possibility of using unverified or outdated price updates.
+    /// as it allows for the possibility of using unverified, outdated, or arbitrary window length twap updates.
     pub fn get_twap_unchecked(
         &self,
         feed_id: &FeedId,
@@ -101,32 +102,38 @@ impl TwapUpdate {
         );
         Ok(self.twap)
     }
-
-    /// Get a `TwapPrice` from a `TwapUpdate` account for a given `FeedId` no older than `maximum_age`.
+    /// Get a `TwapPrice` from a `TwapUpdate` account for a given `FeedId` no older than `maximum_age` with a specific window size.
     ///
     /// # Example
     /// ```
     /// use pyth_solana_receiver_sdk::price_update::{get_feed_id_from_hex, TwapUpdate};
     /// use anchor_lang::prelude::*;
     ///
-    /// const MAXIMUM_AGE : u64 = 30;
+    /// const MAXIMUM_AGE: u64 = 30;
+    /// const WINDOW_SECONDS: u64 = 300; // 5-minute TWAP
     /// const FEED_ID: &str = "0xef0d8b6fda2ceba41da15d4095d1da392a0d2f8ed0c6c7bc0f4cfac8c280b56d"; // SOL/USD
     ///
     /// #[derive(Accounts)]
     /// pub struct ReadTwapAccount<'info> {
     ///     pub twap_update: Account<'info, TwapUpdate>,
     /// }
     ///
-    /// pub fn read_twap_account(ctx : Context<ReadTwapAccount>) -> Result<()> {
+    /// pub fn read_twap_account(ctx: Context<ReadTwapAccount>) -> Result<()> {
     ///     let twap_update = &ctx.accounts.twap_update;
-    ///     let twap = twap_update.get_twap_no_older_than(&Clock::get()?, MAXIMUM_AGE, &get_feed_id_from_hex(FEED_ID)?)?;
+    ///     let twap = twap_update.get_twap_no_older_than(
+    ///         &Clock::get()?,
+    ///         MAXIMUM_AGE,
+    ///         WINDOW_SECONDS,
+    ///         &get_feed_id_from_hex(FEED_ID)?
+    ///     )?;
     ///     Ok(())
     /// }
     /// ```
     pub fn get_twap_no_older_than(
         &self,
         clock: &Clock,
         maximum_age: u64,
+        window_seconds: u64,
         feed_id: &FeedId,
     ) -> std::result::Result<TwapPrice, GetPriceError> {
         // Ensure the update isn't outdated
@@ -138,6 +145,14 @@ impl TwapUpdate {
                 >= clock.unix_timestamp,
             GetPriceError::PriceTooOld
         );
+
+        // Ensure the twap window size is as expected
+        let actual_window = twap_price.end_time.saturating_sub(twap_price.start_time);
+        check!(
+            actual_window == i64::try_from(window_seconds).unwrap(),
+            GetPriceError::InvalidWindowSize
+        );
+
         Ok(twap_price)
     }
 }
@@ -543,13 +558,12 @@ pub mod tests {
             Err(GetPriceError::MismatchedFeedId)
         );
     }
-
     #[test]
     fn test_get_twap_no_older_than() {
         let expected_twap = TwapPrice {
             feed_id: [0; 32],
             start_time: 800,
-            end_time: 900,
+            end_time: 900, // Window size is 100 seconds (900 - 800)
             price: 1,
             conf: 2,
             exponent: -3,
@@ -571,15 +585,27 @@ pub mod tests {
         // Test unchecked access
         assert_eq!(update.get_twap_unchecked(&feed_id), Ok(expected_twap));
 
-        // Test with age check
+        // Test with correct window size (100 seconds)
         assert_eq!(
-            update.get_twap_no_older_than(&mock_clock, 100, &feed_id),
+            update.get_twap_no_older_than(&mock_clock, 100, 100, &feed_id),
             Ok(expected_twap)
         );
 
+        // Test with incorrect window size
+        assert_eq!(
+            update.get_twap_no_older_than(&mock_clock, 100, 101, &feed_id),
+            Err(GetPriceError::InvalidWindowSize)
+        );
+
+        // Test with incorrect window size
+        assert_eq!(
+            update.get_twap_no_older_than(&mock_clock, 100, 99, &feed_id),
+            Err(GetPriceError::InvalidWindowSize)
+        );
+
         // Test with reduced maximum age
         assert_eq!(
-            update.get_twap_no_older_than(&mock_clock, 10, &feed_id),
+            update.get_twap_no_older_than(&mock_clock, 10, 100, &feed_id),
             Err(GetPriceError::PriceTooOld)
         );
 
@@ -589,7 +615,7 @@ pub mod tests {
             Err(GetPriceError::MismatchedFeedId)
         );
         assert_eq!(
-            update.get_twap_no_older_than(&mock_clock, 100, &mismatched_feed_id),
+            update.get_twap_no_older_than(&mock_clock, 100, 100, &mismatched_feed_id),
             Err(GetPriceError::MismatchedFeedId)
         );
     }