refactor: address pr comments

Author: tejasbadadare

target_chains/solana/cli/src/main.rs

@@ -394,7 +394,7 @@ pub fn process_write_encoded_vaa_and_post_price_update(
     let encoded_vaa_keypair = Keypair::new();
 
     // Transaction 1: Create and initialize VAA
-    let init_instructions = create_and_init_encoded_vaa_instructions(
+    let init_instructions = init_encoded_vaa_and_write_initial_data_ixs(
         &payer.pubkey(),
         vaa,
         &wormhole,
@@ -410,7 +410,7 @@ pub fn process_write_encoded_vaa_and_post_price_update(
     let price_update_keypair = Keypair::new();
     let mut update_instructions = vec![ComputeBudgetInstruction::set_compute_unit_limit(600_000)];
 
-    update_instructions.extend(write_and_verify_vaa_instructions(
+    update_instructions.extend(write_remaining_data_and_verify_vaa_ixs(
         &payer.pubkey(),
         vaa,
         &encoded_vaa_keypair.pubkey(),
@@ -457,7 +457,7 @@ pub fn process_write_encoded_vaa_and_post_twap_update(
     let end_encoded_vaa_keypair = Keypair::new();
 
     // Transaction 1: Create and initialize start VAA
-    let start_init_instructions = create_and_init_encoded_vaa_instructions(
+    let start_init_instructions = init_encoded_vaa_and_write_initial_data_ixs(
         &payer.pubkey(),
         start_vaa,
         &wormhole,
@@ -470,7 +470,7 @@ pub fn process_write_encoded_vaa_and_post_twap_update(
     )?;
 
     // Transaction 2: Create and initialize end VAA
-    let end_init_instructions = create_and_init_encoded_vaa_instructions(
+    let end_init_instructions = init_encoded_vaa_and_write_initial_data_ixs(
         &payer.pubkey(),
         end_vaa,
         &wormhole,
@@ -484,13 +484,13 @@ pub fn process_write_encoded_vaa_and_post_twap_update(
 
     // Transaction 3: Write remaining VAA data and verify both VAAs
     let mut verify_instructions = vec![ComputeBudgetInstruction::set_compute_unit_limit(400_000)];
-    verify_instructions.extend(write_and_verify_vaa_instructions(
+    verify_instructions.extend(write_remaining_data_and_verify_vaa_ixs(
         &payer.pubkey(),
         start_vaa,
         &start_encoded_vaa_keypair.pubkey(),
         wormhole,
     )?);
-    verify_instructions.extend(write_and_verify_vaa_instructions(
+    verify_instructions.extend(write_remaining_data_and_verify_vaa_ixs(
         &payer.pubkey(),
         end_vaa,
         &end_encoded_vaa_keypair.pubkey(),
@@ -523,7 +523,7 @@ pub fn process_write_encoded_vaa_and_post_twap_update(
 }
 
 /// Creates instructions to initialize an encoded VAA account and write the first part of the VAA data
-pub fn create_and_init_encoded_vaa_instructions(
+pub fn init_encoded_vaa_and_write_initial_data_ixs(
     payer: &Pubkey,
     vaa: &[u8],
     wormhole: &Pubkey,
@@ -577,7 +577,7 @@ pub fn create_and_init_encoded_vaa_instructions(
 }
 
 /// Creates instructions to write remaining VAA data and verify the VAA
-pub fn write_and_verify_vaa_instructions(
+pub fn write_remaining_data_and_verify_vaa_ixs(
     payer: &Pubkey,
     vaa: &[u8],
     encoded_vaa_keypair: &Pubkey,

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

@@ -485,6 +485,9 @@ fn post_twap_update_from_vaas<'info>(
     // Calculate the TWAP and store it in the output account
     match (start_message, end_message) {
         (Message::TwapMessage(start_msg), Message::TwapMessage(end_msg)) => {
+            // Verify that the feed ids and expos match, the start msg was published before the end msg,
+            // and that they are the first messages within their slots
+            validate_twap_messages(&start_msg, &end_msg)?;
             let (price, conf, down_slots_ratio) = calculate_twap(&start_msg, &end_msg)?;
 
             twap_update_account.write_authority = write_authority.key();
@@ -504,7 +507,7 @@ fn post_twap_update_from_vaas<'info>(
     Ok(())
 }
 
-fn calculate_twap(start_msg: &TwapMessage, end_msg: &TwapMessage) -> Result<(i64, u64, u32)> {
+fn validate_twap_messages(start_msg: &TwapMessage, end_msg: &TwapMessage) -> Result<()> {
     // Validate feed ids match
     require!(
         start_msg.feed_id == end_msg.feed_id,
@@ -532,6 +535,12 @@ fn calculate_twap(start_msg: &TwapMessage, end_msg: &TwapMessage) -> Result<(i64
         end_msg.prev_publish_time < end_msg.publish_time,
         ReceiverError::InvalidTwapEndMessage
     );
+    Ok(())
+}
+
+/// Calculate the TWAP for the window before start and end messages
+/// Warning: The parameters aren't checked for validity, call `validate_twap_messages` before using.
+fn calculate_twap(start_msg: &TwapMessage, end_msg: &TwapMessage) -> Result<(i64, u64, u32)> {
     let slot_diff = end_msg
         .publish_slot
         .checked_sub(start_msg.publish_slot)
@@ -669,8 +678,8 @@ fn verify_vaa_data_source(
 }
 
 #[cfg(test)]
-/// Unit tests for the core TWAP calculation logic in `calculate_twap`
-/// This test module is here because `calculate_twap` is private and can't
+/// Unit tests for the core TWAP calculation logic in `calculate_twap` and `validate_twap_messages`
+/// This test module is here because these functions are private and can't
 /// be imported into `tests/test_post_twap_updates`.
 mod calculate_twap_unit_tests {
     use super::*;
@@ -698,6 +707,7 @@ mod calculate_twap_unit_tests {
         let start = create_basic_twap_message(100, 100, 90, 1000);
         let end = create_basic_twap_message(300, 200, 180, 1100);
 
+        validate_twap_messages(&start, &end).unwrap();
         let price = calculate_twap(&start, &end).unwrap();
         assert_eq!(price.0, 2); // (300-100)/(1100-1000) = 2
     }
@@ -707,7 +717,7 @@ mod calculate_twap_unit_tests {
         let start = create_basic_twap_message(100, 100, 90, 1100);
         let end = create_basic_twap_message(300, 200, 180, 1000);
 
-        let err = calculate_twap(&start, &end).unwrap_err();
+        let err = validate_twap_messages(&start, &end).unwrap_err();
         assert_eq!(err, ReceiverError::InvalidTwapSlots.into());
     }
 
@@ -716,13 +726,13 @@ mod calculate_twap_unit_tests {
         let start = create_basic_twap_message(100, 100, 110, 1000);
         let end = create_basic_twap_message(300, 200, 180, 1100);
 
-        let err = calculate_twap(&start, &end).unwrap_err();
+        let err = validate_twap_messages(&start, &end).unwrap_err();
         assert_eq!(err, ReceiverError::InvalidTwapStartMessage.into());
 
         let start = create_basic_twap_message(100, 100, 90, 1000);
         let end = create_basic_twap_message(300, 200, 200, 1100);
 
-        let err = calculate_twap(&start, &end).unwrap_err();
+        let err = validate_twap_messages(&start, &end).unwrap_err();
         assert_eq!(err, ReceiverError::InvalidTwapEndMessage.into());
     }
 
@@ -731,6 +741,7 @@ mod calculate_twap_unit_tests {
         let start = create_basic_twap_message(i128::MIN, 100, 90, 1000);
         let end = create_basic_twap_message(i128::MAX, 200, 180, 1100);
 
+        validate_twap_messages(&start, &end).unwrap();
         let err = calculate_twap(&start, &end).unwrap_err();
         assert_eq!(err, ReceiverError::TwapCalculationOverflow.into());
     }
@@ -741,7 +752,7 @@ mod calculate_twap_unit_tests {
         let mut end = create_basic_twap_message(300, 200, 180, 1100);
         end.feed_id = [1; 32];
 
-        let err = calculate_twap(&start, &end).unwrap_err();
+        let err = validate_twap_messages(&start, &end).unwrap_err();
         assert_eq!(err, ReceiverError::FeedIdMismatch.into());
     }
 
@@ -751,7 +762,7 @@ mod calculate_twap_unit_tests {
         let mut end = create_basic_twap_message(300, 200, 180, 1100);
         end.exponent = 9;
 
-        let err = calculate_twap(&start, &end).unwrap_err();
+        let err = validate_twap_messages(&start, &end).unwrap_err();
         assert_eq!(err, ReceiverError::ExponentMismatch.into());
     }
 }

target_chains/solana/pyth_solana_receiver_sdk/src/price_update.rs

@@ -61,11 +61,8 @@ impl PriceUpdateV2 {
 }
 /// A time weighted average price account.
 /// This account is used by the Pyth Receiver program to store a TWAP update from a Pyth price feed.
-///
-/// ## Warning
-/// This function doesn't check whether the TwapUpdate is constructed from verified price updates.
-///     TwapUpdates should be only created and used after the client has already verified the VAAs via the Wormhole contract.
-///     Check out `target_chains/solana/cli/src/main.rs` for an example of how to do this.
+/// TwapUpdates can only be created after the client has verified the VAAs via the Wormhole contract.
+/// Check out `target_chains/solana/cli/src/main.rs` for an example of how to do this.
 ///
 /// It contains:
 /// - `write_authority`: The write authority for this account. This authority can close this account to reclaim rent or update the account to contain a different TWAP update.